BIND 10 #216: Xfrin: Implement the feature items in TODO file.

[BIND 10 Development]

#216: Xfrin: Implement the feature items in TODO file.
------------------------------+---------------------------------------------
      Reporter:  zhanglikun   |        Owner:  shentingting        
          Type:  enhancement  |       Status:  reviewing           
      Priority:  major        |    Milestone:  y2 6 month milestone
     Component:  xfrin        |   Resolution:                      
      Keywords:               |    Sensitive:  0                   
Estimatedhours:  0.0          |        Hours:                      
      Billable:  1            |   Totalhours:                      
      Internal:  0            |  
------------------------------+---------------------------------------------

Comment(by jinmei):

 Replying to [comment:11 shentingting]:
 > Replying to [comment:10 jinmei]:
 >     Now I am thinking about adding test case problem. for
 xfrinConnection.send/recv/_loop/close are copied from asyncore library,
 and those functions are all called based socket API. so I don`t think it
 is  necessary to add new test case for those function.
 >
 We do need tests for these.  Tests are not only for checking they work
 correctly today, but also for ensuring no (or less if any) regression
 happens with future changes.

 I understand it's not straightforward to test code using low level API
 such as socket I/O (if that's what you mean by "those functions are all
 called based socket API").  But it's possible via some indirection
 techniques I already mentioned, and it's important because behaviors
 around socket I/O is a core part of network applications.

 Please add tests.

 > why did we switch from asyncore to select?
 >
 >    There are several reasons:
 >     1. It is not necessary to use asyncore, since each thread is
 responsible for a single xfr connection, socket operations can safely
 block (with timeouts).  this should be easily implemented using the bear
 socket module, and the code would look like more straightforward by
 avoiding complicated logic for asynchrony.
 >     2.  we'd need an explicit communication channel (e.g. a socket pair)
 between the parent thread and xfr connection thread, through which a
 shutdown notification would be sent to the child.  With this approach each
 thread needs to watch at least two channels, and then it would need some
 asynchronous communication mechanism. But the socket pair is not
 asyncore.dispatcher type or its subtype, and it is difficult to switch it
 into a type that asyncore can listen. So it can not put this socket pair
 into map, the notify channel is difficult to build. Select can compelte
 the same function as asyncore in xfrin deamon, and can build a new notify
 channel to deal with quickly terminate on shutdown command. So I switch
 from asyncore to select.
 >     3. The asyncores increases complexity, it is more difficult to read
 code.
 >
 Okay.  The second point makes sense (although I've not confirmed if it's
 impossible with asyncore - but I trust you:-).  Such an implementation
 decision isn't obvious to others, so please document it in the form of
 code comment explicitly.

 (While I have no opinon/preference on asyncore vs select per se) The first
 and third reasons aren't convincing to me, btw.  In fact, the first point
 doesn't make sense because the resulting code with select actually uses
 the asynchronous nature of select.  Arguments on complexity (the third
 point) are often a matter of taste, and may not always be a tie-breaking
 reason if that's a controversial choice.

 Anyway I'm not disagreeing with the change itself.  I just wondered why,
 and I just want the decision to be documented properly.

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