BIND 10 #2922: enhance group subscriber management in msgq

[BIND 10 Development]

#2922: enhance group subscriber management in msgq
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  pselkirk
            Priority:  high          |                       Status:
           Component:  msgq          |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130611
         Sub-Project:  Core          |                   Resolution:
Estimated Difficulty:  4             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  shared memory data source
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => pselkirk


Comment:

 Hello

 Replying to [comment:11 pselkirk]:
 > I'm also not sure about the timing in `wait_msgq`. If I understand
 correctly, it sends up to 10 messages at 100ms intervals, then gives up.
 Will this cause problems on older/slower hardware, or say if bind10 is
 starting up at the same time as everything else on the system?

 No, it sets up to 10 times the `wait_time`, so we wait at most `wait_time`
 seconds, which is 10 by default. This is the same as waiting for the whole
 cfgmgr to start up. But the action we wait for is much more lightweight
 (we don't wait for whole process to start up, just the startup of cfgmgr
 to bubble through the other thread of msgq). So it should pose no
 problems.

 But we still need to wait for it, so the other started processes can
 assume msgq has a session already started.

 > '''src/bin/bind10/tests/init_test.py.in'''
 > I'm not sure why you import `ProcessStartError`, since the only other
 place it's referenced in this module, it's qualified with `init`.
 >
 > This also means you've got this line:
 > {{{
 >         self.assertRaises(ProcessStartError, init.wait_msgq)
 > }}}
 > where both `ProcessStartError` and `wait_msgq` are from `init`, but one
 is qualified and one unqualified.

 This is slightly more complex than you seem to think. The init in this
 snippet is actually a local variable holding the tested object, not the
 top-level module. So I had to import the ProcessStartError into the global
 scope, because init.ProcessStartError did _not_ refer to the class at that
 scope.

 If it is deemed too confusing, I probably could rename the init variable
 to something else.

 > What sort of trouble? Is this something we should be testing for?

 I added a description.

 It is tested ‒ that's why it isn't a unit test, but whole-process test, to
 test the thread interaction.

 > I scratch my head about the usage of single vs double quotes.

 Actually, I just don't think much about the quotes. I have the habit from
 perl, where it is good to use both kinds, since each have different
 meaning. Sometimes in other languages which have the same meaning for both
 kinds, I tend to put machine-readable strings to single-quotes and human-
 readable ones to double ones, but as I don't think about them, I just mix
 them mostly randomly.

 I at least managed to have the same kind of quote at the beginning of the
 same string as at the end O:-). I tried to unify them.

 > '''src/bin/msgq/tests/msgq_test.py'''
 > `MsgQTest.test_notifies` should probably test unsubscription from a
 group not prevously subscribed to.

 OK

 > `test_notifies` and `test_notifies_implicit_kill` have some ugly code to
 "Steal the lname". Is there a reason you can't use `fd_to_lname`?

 The reason is I added the `fd_to_lname` later. I changed it, but it
 doesn't seem to be _much_ cleaner, just a tiny bit.

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