BIND 10 #352: Improve Xfrout request handling strategy

[BIND 10 Development]

#352: Improve Xfrout request handling strategy
------------------------------+---------------------------------------------
      Reporter:  zzchen_pku   |        Owner:  zhanglikun
          Type:  enhancement  |       Status:  reviewing 
      Priority:  major        |    Milestone:            
     Component:  xfrout       |   Resolution:            
      Keywords:               |    Sensitive:  0         
Estimatedhours:  0.0          |        Hours:  0         
      Billable:  1            |   Totalhours:  1.5       
      Internal:  0            |  
------------------------------+---------------------------------------------

Comment(by jinmei):

 Replying to [comment:11 zhanglikun]:

 > why and when ==
 > {{
 >     Mix-In class to override the function serve_forever()
 >     and shutdown() in class socketserver::TCPServer.
 >       serve_forever() in socketserver.TCPServer use polling
 >     for checking request, which reduces the responsiveness to
 >     the shutdown request and wastes cpu at all other times.
 > }}
 >
 > I can't find any better words to describe this mixin, I would appreciate
 your better words for 'why and when' if you can give. thanks.
 >
 I've committed suggested text (r3324), and I have related additional
 suggestions:
  - I suggest the name of the mix-in class to NoPollMixIn (documentation
 was revised with the new name) because "ServeMixIn" is too generic.  IMO
 the class name should briefly indicate the role of the class.
  - I also suggest change the module name from serve_mixin to
 socketserver_mixin so that it will be clear this mixin will be used with
 the socketserver module.

 And one related comment: I believe what this class overrides is actually
 "BaseServer" instead of "TCPServer" because, as far as I can see, none of
 the mix-in code assume specific part of TCPServer.  Suggested
 documentation was updated on that point, too.

 Please feel free to update the suggested doc further.  In particular,
 whether you agree with the suggested class name or not, please make the
 documentation and the actual class name consistent (which are currently
 not).

 > > I'm not sure if continue is the correct behavior.  I actually don't
 know what kind of error other than EINTR is expected in this context, but
 if it's a permanent error (e.g. due to some broken state in the socket),
 the error will repeat and this loop continues infinitely (until we get a
 shutdown message).  We should clarify which errors we can receive here,
 and continue only for transient errors (which may be none or all).
 >
 > I also don't know what kind of error other than EINTR will happen, but
 this time I change the code as print the error and break. Not sure it's
 the best way.
 >
 I think just do 'break' is okay for now, but don't think it a good idea to
 dump the error to stderr since we cannot assume how this mixin is used.
 Ideally, the serve_forever() thread sends an exit code to the waiting
 thread, and the waiting thread raises an exception if something went
 wrong.  But that may be too much at the moment.  So we should probably
 simply remove this line for now:
 {{{
                     sys.stderr.write("Error with select(), %s\n", err)
 }}}

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