BIND 10 #1751: stats doesn't separate or consolidate reports from multiple auth instances

[BIND 10 Development]

#1751: stats doesn't separate or consolidate reports from multiple auth instances
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  vorner
  jinmei                             |                Status:  reviewing
                       Type:         |             Milestone:
  defect                             |  Sprint-20120320
                   Priority:         |            Resolution:
  medium                             |             Sensitive:  0
                  Component:         |           Sub-Project:  Core
  statistics                         |  Estimated Difficulty:  6
                   Keywords:         |           Total Hours:  0
            Defect Severity:  N/A    |
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by naokikambe):

 * owner:  naokikambe => vorner


Comment:

 Thank you for reviewing.

 Replying to [comment:10 vorner]:
 > Yes, I agree with this ‒ I think it is better than not having it, but it
 still needs more tweaking. I'm especially worried about one thing ‒ now a
 module that has multiple instances preserves them across restarts, while
 the others don't. This acts in an inconsistent way. Would you create a
 ticket for it?

 Actually I'm not sure whether accumulated counts which the non-existent
 instance had should be lost, but I revised the codes so that such
 inconsistency wouldn't happen. Stats queries the pid and instance list to
 Boss, searches the non-existent instance, and then removes statistics data
 which it had. Please review 94f99cad8539239a93877a1774fbadf89238c181.

 > As for the thing with PIDs, we probably want to solve it in a more wide
 way, together with addressing multiple instances in commands, etc. So
 that'll probably provide further insight on how this should be handled in
 the long term.

 Yes, I think using PID is a short-term solution. Of course, we should
 discuss further about such addressing.

 > About the code, first I'd like to note that resolver also supports
 multiple instances. Would you add the support to it as well?

 AFAIK, query counters in the resolver aren't implemented yet.
 Maybe auth/statistics.{cc,h} are reusable for the resolver. They should be
 revised for the resolver, but we can implement counters by the same way as
 auth, e.g. #1399, #1401 and #1613.
 Of course, query counts in a multi-instance resolver would be also
 consistent after that implementation.

 > The tests of stats, I think `EXPECT_GT` would be better here:
 > {{{#!c++
 >     EXPECT_TRUE(statistics_session_.sent_msg->get("command")
 >                 ->get(1)->get("pid")->intValue() > 0);
 > }}}

 Thank you for the suggestion, but I replaced it with the check with a real
 pid.
 Please review ddf22289ac033aa2ffdaf8e348c4a05f2d1d6951.

 > What do the two stars mean here?
 > {{{#!python
 > **{'queries.tcp':1001}
 > }}}

 I just intended to pass a keyword argument to the function. The keyword
 includes "." , which has a special meaning in python, so the keyword must
 be literal.

 > I think you mean `set Boss/components/b10-auth-2` here:
 > {{{
 > echo 'config add Boss/components b10-auth-2
 > config set Boss/components/b10-auth { "special": "auth", "kind":
 "needed" }
 > }}}

 Yes, that was my typo. But after I fixed it, maybe I encountered another
 bug. After the second auth is removed via bindctl, the auth doesn't
 respond any more. It doesn't respond even to shutdown via bindctl. This
 seems for me to be the same phenomenon in #1703. So I gave up the final
 statistics check there.
 And I also updated the regression check against an probable statistics
 inconsistency displayed by `Stats show'. Two more auths are started and
 the query counters are checked multiple times. Please review
 3ca2c002d807fa1f8e7dbf6731ac4f1796f4e5a0.

 > Also, would you provide a changelog entry? I think one would be needed,
 since it fixes a visible problem.

 Yes, I would. Please review eecc0d847fc5f9dc74ce96788e0b3489a98fb244.

 Thanks

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