BIND 10 #2209: define and implement ConfigurableClientList::getCacheZoneUpdater()

BIND 10 Development do-not-reply at isc.org
Fri Oct 26 21:04:43 UTC 2012


#2209: define and implement ConfigurableClientList::getCacheZoneUpdater()
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20121106
  medium                             |            Resolution:
                  Component:  data   |             Sensitive:  0
  source                             |           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):

 Replying to [comment:13 vorner]:

 > > I'm not sure about the latter, but as I said I can live with it as an
 > > intermediate workaround.  For the former, I think we need to run one
 > > more review cycle after the cleanup.  BTW, #2208 now seems to be ready
 > > for merge.
 >
 > I tried merging it. The changes are pushed now. The merge was little bit
 > interesting. The merge commit contains nothing substantial, just the
 conflict
 > resolution and some slight cleanups (constification) I noticed on the
 way. The
 > adjustments to tests and the code are in follow-up commits.
 >
 > I got rid of the getZoneTableSegment too.

 The merge and after merge changes look basically okay.  I also
 confirmed the previous concerns were resolved.

 Some minor comments:
 - I suggest renaming `DataSourceInfo::segment_` something like
   `ztable_segment_` because "segment" can be used in several different
   contexts.  Same for the constructor parameter, etc, too.
 - I guess we can now remove `ConfigurableClientList::mem_sgmt_`.
 - I'd leave the same (sense of) note about the use of assert()
   in the `ZoneTableSegmentLocal` destructor as this one:
 {{{#!cpp
     // leak.  assert() (with abort on failure) may be too harsh, but
     // it's probably better to find more leaks initially.  Once it's
 stabilized
     // we should probably revisit it.
 }}}
   which is currently in the `ConfigurableClientList` destructor (and
   will be removed if we adopt the second suggestion)

 With these I think the branch can be merged.

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


More information about the bind10-tickets mailing list