BIND 10 #2157: Create an interface to pass statistics counters in Auth module

BIND 10 Development do-not-reply at isc.org
Fri Dec 21 17:27:20 UTC 2012


#2157: Create an interface to pass statistics counters in Auth module
-------------------------------------+-------------------------------------
            Reporter:  y-aharen      |                        Owner:
                Type:  enhancement   |  jinmei
            Priority:  medium        |                       Status:
           Component:  b10-auth      |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130108
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  8             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Comments on the rest of the branch.  This completes this round of
 my revised review.

 '''counter_dict.h'''

 - there are some unnecessary ';'s (also in counter.h).  I removed them
   myself.

 Unrelated to this branch:
 - counter_dict.h should have more documentation.
 - (partly because of the lack of doc) I'm not sure about the intended
   usage of this file and `CounterDictionary`, but in general, we
   should use an unnamed namespace in a header file:
 {{{#!cpp
 namespace {
 typedef boost::shared_ptr<isc::statistics::Counter> CounterPtr;
 typedef std::map<std::string, CounterPtr> DictionaryMap;
 }
 }}}

 '''(lettuce) queries.feature'''

 It took quite a long time to complete this feature (almost 2 minutes
 in my environment), and despite the cost most test cases seem to be
 boring (repeating the same cases which are quite likely to succeed if
 others do).  While the duration of the test is less critical in system
 tests, the cost-benefit balance with the current setup doesn't seem to
 be reasonable.  Can't it be more sophisticated?

 For example, if we expect only a few counters to have non 0 values,
 I'd unify this condition into a single line of condition rather than
 repeating things like this:
 {{{
         Then the statistics counter v6 in the category request for the
 zone _SERVER_ should be 0
 }}}

 And focus on covering more interesting cases than repeating the same
 boring statements again and again.

 '''bindctl/tests.sh'''

 If I understand it correctly, the statistics related part of this test
 is reasonably covered by lettuce tests like queries.feature
 (correct?).  Since the bindctl test has another, non-statistics
 related issue, and is now disabled in master, I'd rather remove
 statistics related checks from it at this stage than tweaking it to
 make work with additional hacks.  See also #2568.

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


More information about the bind10-tickets mailing list