BIND 10 #2205: introduce a "data source configurator" thread in auth

[BIND 10 Development]

#2205: introduce a "data source configurator" thread in auth
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20121023
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  b10-auth                           |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  5
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
  background zone loading            |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Thanks for the review.

 Replying to [comment:15 jelte]:

 > The DataSrcClientsBuilderTest.start test segfaults for me (in
 shutdownCheck); i fear the std::list object is cleared when the clientsmgr
 is destroyed (so the pointer is still there but size() runs into trouble),
 if this works for you i do wonder what valgrind thinks about it :)

 Ah, yes, you're right.  It was obviously wrong.

 I couldn't come up with a very clean solution either, and tried to
 address it at commit c2608e7.  I believe it should at least solve the
 crash issue.

 > datasrc_clients_mgr.h:
 >
 > - DataSrcClientsMgrBase destructor comment: "this could time" I think
 you meant "this could take a long time"

 Ah, right, fixed.

 > - the DataSrcClientsMgrBase destructor logs on normal exception, but not
 on the catchall. Those shouldn't happen, but should we not log there too?

 I thought it might rather make sense to eliminate the possibility of
 having another exception, but since the risk is already there, I'm
 okay with logging it too.  Added.

 > - #ifdef notyet   A fine approach to 'temporary dead code with plan to
 fix', should we standardize on this method? (up to now i have tended to
 use '#if 0', and that is what i'd grep for if i was looking for dead code)

 Standardizing it sounds like a good idea.  I'd suggesting something
 unique to us in that case, e.g., ifdef BIND10_TODO.  But in any case
 it would be beyond the scope of this ticket (and in this particular
 case this ifdef should really be removed very soon anyway).  So I
 didn't touch it.

 > {{{
 > /// This class is a server of \c DataSrcClientsMgr.  Except for tests,
 > /// applications should not directly access to this class.
 > }}}
 > I don't really understand this comment (and the second line has an
 error, it misses 'have')

 Okay, I've updated the whole paragraph.

 > auth_messages.mes: you forgot to reorder it :)

 Ah, right, thanks.  I intended to reorder it but forgot.

 > test_datasrc_clients_mgr.h: I know these shouldn't happen anyway, and
 are in tests, so this is a really trivial point, but the two isc_throw
 strings could use a slightly better text

 Okay, I tried to make it better.

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