BIND 10 #1924: Msgq undeliverable notifications

[BIND 10 Development]

#1924: Msgq undeliverable notifications
-------------------------------------+-------------------------------------
            Reporter:  vorner        |                        Owner:
                Type:  task          |  jinmei
            Priority:  medium        |                       Status:
           Component:  msgq          |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130219
         Sub-Project:  Core          |                   Resolution:
Estimated Difficulty:  6             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:  msgq-
                                     |  ng
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:19 vorner]:

 First off, it would be helpful if you refer to other original comments
 with a response of whether or not it's addressed (and if necessary,
 why), or at least with a summary sentence that would read "I believe
 I've addressed all other points".  If none of these were commented, I
 cannot be sure if the revise is ongoing (but the ticket was returned
 to me to peek into it) or is supposed to be complete.  Also, I cannot
 be sure if some points are not addressed it was intentional or just
 overlooked.

 > Replying to [comment:17 jinmei]:
 > > In that case I believe it should be "the senders that hunger for
 > > response".
 >
 > I meant hunger as noun, not a verb. But I changed the whole thing
 anyway, so it should be clearer.
 >
 > > - isc::util doesn't seem to be an appropriate place to define them.
 > >   "CC" is specific to the cc module, while util is generally expected
 > >   to be (kitchensink) placeholder for things that don't belong to
 > >   anything else.  It's as awkward as seeing e.g. b10-auth log message
 > >   IDs are defined in isc::util.
 >
 > I'm not sure. I thought the file could be used for other constants too,
 in future. Like some protocols between auth and XFROUT and such. Or do you
 think it is better to have separate constants file for each of these? Then
 we need to put the generator scripts somewhere common, though, and I'm not
 sure where.

 This type of thing is ultimately a matter of opinion, so it can vary,
 but my specific suggestion is:

 - place (both) the generator scripts under src/lib/util/python/.  the
   current directories are basically intended to place things related
   to run-time library, not for compile time tools, and don't seem to
   be the best choice to me.
 - place common_defs.cc under src/lib/cc, probably renaming it e.g.
   (cc/)proto_defs.cc).  Right now all definitions are related to
   the core CC protocol, so this place seems to be the best to have it.
   The actual definition will be part of libb10-cc, and that should be
   necessary and sufficient because all apps that need these defs
   should need libb10-cc, and apps don't need to use cc (pure libdns++
   apps, e.gg) doesn't need to have it.  corresponding .h will be here,
   too.
 - generate the python file from .cc under src/lib/python/isc/cc for
   the same reason.

 In future, when application specific constants will have to be
 defined, I guess we'll place the original definition with the "primary
 owner" of it.  For example, the "loadzone" command name for auth will
 be defined in src/bin/auth.  Where to generate Python counterparts
 will have to be figured out; I don't know what's the best right now,
 but I don't think it a bad idea to introduce a dedicate python package
 under src/lib/python/isc for such purposes, or, just like we do for
 log messages, define something like src/lib/python/isc/app_constants/
 where we'll have auth_command_defs.py, etc.

 > > - I'm not so sure if handling partial send failure (there are multiple
 > >   possible recipients and a subset of them fail) as an "OK" case; if
 > >   xfrin sends something to multiple instances of auth and not all of
 > >   them were correctly delivered (but the failing auth is still somehow
 > >   running), the system will be in an inconsistent state.  That said,
 > >   this level of discussion is probably beyond the scope of this ticket
 > >   and should go to the larger msgq revisit work.  So I'm making it
 > >   just as a comment, not a request for changing the code.
 >
 > This is not about consistency. This is about the fact that when one
 process wants to call a remote function to get some information, it needs
 to wait for an answer. This is to provide an answer that no real data will
 ever come. Generally, the expected number of recipients would be one. Even
 if there are more and one of them is not receiving, the others will
 provide answer, so this is correct. The parameter is not called
 „must_deliver_to_all“, but „want_answer“.
 >
 > Also, what you describe is probably a notification that does not need
 any kind of answer at all anyway.
 >
 > And, finally, if there was EPIPE or something from the recipient, it
 means it is probably already dead. If it isn't, it will be soon, because
 our modules can't live without the CC connections and if they lose it,
 they usually just crash (and get restarted).
 >
 > So, from these points, I believe the current behaviour is the more
 correct one.

 I was probably not clear enough, but my point was higher than whether
 this specific case is okay or not - in fact, I didn't necessarily
 suggest changing that.  My concern is that we don't have a clear big
 picture of the entire communication model/protocol with msgq; whether
 it's one-to-one or one-to-many, what kind of failure mode is provided,
 which level of reliability is provided (or not provided), etc.  Since
 these are not sufficiently clarified, we always have questions about
 unclear cases like the xfrin-auth above.  We just don't know whether
 this scenario fits this extension or not because we don't have the big
 picture.  Just adding some random extension without clarifying it
 seems to be too ad hoc to me, just seemingly addressing some specific
 cases, leaving many open questions and even creating new issues.

 That said...I expect (or hope) we'll clarify these points and revisit
 the msgq system at a more fundamental way, and, with that assumption,
 I wouldn't argue on this point in the context of this ticket.

 One other comment on the branch.

 '''msgq_test.py'''
 - hmm, I still can't parse this sentence:
 {{{
         be mostly independant (eg. if the recipient is missing, it
         shouldn't matter by which means it was discovered the recipient
         is missing to how we hendle the value in reply header). If
 }}}
   especially after the "to how we..." part.  besides, "hendle" should
   be "handle"?

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