BIND 10 #347: Implement query counters in auth module

[BIND 10 Development]

#347: Implement query counters in auth module
------------------------------+---------------------------------------------
      Reporter:  naokikambe   |        Owner:  y-aharen             
          Type:  enhancement  |       Status:  reviewing            
      Priority:  major        |    Milestone:  y2 12 month milestone
     Component:  b10-auth     |   Resolution:                       
      Keywords:               |    Sensitive:  0                    
Estimatedhours:  0.0          |        Hours:  0                    
      Billable:  1            |   Totalhours:  0                    
      Internal:  0            |  
------------------------------+---------------------------------------------
Changes (by jinmei):

  * owner:  jinmei => y-aharen


Comment:

 Replying to [comment:8 y-aharen]:

 > > '''stats.cc'''
 > >   - The comment and log messages assume that the other end is the
 > >     b10-stats program, but interface-wise it's not necessarily correct
 > >     (that could be a third-party app that implements the stat API,
 > >     right?)
 > Counter in Auth module is not intended to communicate with third-party
 stats module. It only communicates with b10-stats.
 >
 Hmm, this is different from my general understanding (general = not just
 about statistics): the protocol among various BIND 10 modules is open, and
 any single module can be replaced (and should be replaceable) with other
 implementation, as long as it supports the inter module protocol.  For
 example, we can replace b10-xfrin with a third party xfrin process without
 affecting how the entire BIND 10 system behaves.  And, there shouldn't be
 a reason why statistics is an exception.

 It's possible my understanding is incorrect, however, so, if you don't
 agree please bring it to bind10-dev.

 > >   - BTW, even if it doesn't cause timeout and exception, we should
 > >     actually not even block in group_recvmsg (and also
 > >     group_sendmsg(), which can happen because it's a connected
 > >     channel): b10-auth shouldn't stop handling DNS queries even for 1
 > >     second.  Since it should relatively a rare event we can live with
 > >     the current blocking mode (and there are actually other parts of
 > >     b10-auth that have the same problem).  But please leave a TODO
 > >     comment on this point.  We should eventually solve it.
 > I don't think that blocking in group_{send,recv}msg also blocks handling
 of DNS queries. I understand blocking read/write operation stops in the
 ASIO library, and if another event arrives ASIO starts handling it. Excuse
 me if I misunderstood.
 >
 This is not true for group_sendmsg(), because it uses a blocking primitive
 of ASIO (asio::write()).

 I actually thought it also applied to group_recvmsg(), but after re-
 checking the code I found (or rediscovered) that it uses a tricky
 asynchronous loop with a timer (internally calling asio::run_one()), so,
 you seem to be right; the internal loop will handle other events while
 waiting for the stats response.

 But I suspect it was a non intended side effect, and the original intent
 was to introduce a timeout for a blocking read operation (if you want to
 be sure, we should ask Jelte, who implemented it).  Actually, calling
 run_one() in an event handler of asio::run() don't seem to be safe in
 general; there seems to be possible race condition in handling a lock for
 the shared io_message object.  Besides, in my understanding we'll
 eventually integrate this code into the generic asio_link wrapper, at
 which point this will probably be a blocking operation again.

 So, basically, I think my point still stands.  Note, however, that I
 didn't request to solve this problem.  This is just a note, and my
 suggestion was to leave some comments for future fixes.

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