BIND 10 #2179: Update Stats to support partial statistics updates (was: Update Stats to support differential statistics updates)

[BIND 10 Development]

#2179: Update Stats to support partial statistics updates
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  naokikambe
  naokikambe                         |                Status:  reviewing
                       Type:         |             Milestone:
  enhancement                        |  Sprint-20120904
                   Priority:         |            Resolution:
  medium                             |             Sensitive:  0
                  Component:         |           Sub-Project:  DNS
  statistics                         |  Estimated Difficulty:  8
                   Keywords:         |           Total Hours:  0
            Defect Severity:  N/A    |
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Old description:

> According to StatsModule#Differentialstatisticsupdates, update the stats
> module to support differential statistics updates.
>
> When the target module returns statistics data, the stats module should
> replace the existent one with the new one if it already has existent
> statistics data of the corresponding name. That is, the method
> ''update_statistics_data'' in {{{stats.py}}} should be updated.
>
> This ticket depends on #2136. So this updating should be done based on
> the branch trac2136 if it hasn't merged with the master branch yet.
>
> [Additional Description]
> * Update the stats module to update statistics items which each target
> module specifies. It doesn't need to discard unspecified item when each
> module replies. It needs to keep it.
>
> * Add tests so that it is clear that ''named_set'' type in a statistics
> spec is acceptable.
>
> * Update the stats module so that a configuration unique identifier, e.g.
> sss/ttt[i], is acceptable for using in a item name as an answer of the
> ''getstats'' command.

New description:

 According to StatsModule#Partialstatisticsupdates, update the stats module
 to support partial statistics updates.

 When the target module returns statistics data, the stats module should
 replace the existent one with the new one if it already has existent
 statistics data of the corresponding name. That is, the method
 ''update_statistics_data'' in {{{stats.py}}} should be updated.

 This ticket depends on #2136. So this updating should be done based on the
 branch trac2136 if it hasn't merged with the master branch yet.

 [Additional Description]
 * Update the stats module to update statistics items which each target
 module specifies. It doesn't need to discard unspecified item when each
 module replies. It needs to keep it.

 * Add tests so that it is clear that ''named_set'' type in a statistics
 spec is acceptable.

 * Update the stats module so that a configuration unique identifier, e.g.
 sss/ttt[i], is acceptable for using in a item name as an answer of the
 ''getstats'' command.

--

Comment (by naokikambe):

 Hello,

 Replying to [comment:10 jelte]:
 > First of all, i'd be clear and talk about 'partial' rather than
 'differential' data.
 > Changelog:
 > - see above (differential->partial)
 > - and a few language improvements; i propose:
 ...
 > And a side note: it's easier to propose a changelog ticket here (on
 trac) rather than through the branch, as it will most certainly cause an
 unnecessary conflict (just add it when you merge), and you'll need to
 update it to get the commit id anyway.
 >
 > But since it's there I have a comment, we use tabs instead of 8 spaces
 :)

 Thank you very much! That proposal is OK for me. I also updated the file
 on the repository for consistency like that. And I also updated the ticket
 summary and description and the wikipage StatsModule: ''differential'' ->
 ''partial''.

 > stats.py.in:
 > in merge_oldnew(), I'd check whether the types are the same, and raise
 an error if they arent (i'm guessing you could have some pretty unexpected
 results if they are not, for some reason, as you then just drop the old).

 I'm not sure we should be so pessimistic. It checks validity by
 validate_statistics() before it examines the values by merge_oldnew(). So
 the types of two values must be same at the time when invoking
 merge_oldnew(). Even if the types are different, such type can be `str` or
 `float` or `int` or `bool` or `None`. In that case, I think it can just
 replace the old with the new without any exception.

 >
 > Also, I wonder if the lists version is useful; if they are of different
 length, can you be sure that the shorter list is the same as the start of
 the longer list?

 I think this case is very rare. If the new list is longer than the old, it
 should just replace the old with the new. Otherwise, if the old list is
 longer than the new, values of the latter indexes of the old list
 shouldn't be discarded, I think. Because that ''partial update'' is
 supported and the existing values not specified in argument should be
 preserved. For example, if we set the following two values:
 {{{#!python
 > old = [1, 2, 3, 4]
 > new = [5, 6]
 }}}
 then, we would get the following merged value:
 {{{#!python
 > stats.merge_oldnew(old, new)
 > [5, 6, 3, 4]
 }}}

 Regards,

-- 
Ticket URL: <http://bind10.isc.org/ticket/2179#comment:11>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development