BIND 10 #2851: extensions to ConfigurableClientList::getCachedZoneWriter

[BIND 10 Development]

#2851: extensions to ConfigurableClientList::getCachedZoneWriter
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  jinmei
            Priority:  medium        |                       Status:
           Component:  data source   |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130514
         Sub-Project:  DNS           |                   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 => jinmei


Comment:

 Hello

 Few comments to the code. I didn't find a test testing reload after the
 zone was missing and then later it was added to the underlying data
 source. There's test checking it does not fail, but I don't think I
 noticed one that would try to add the zone and load it later.

 I believe this is either wrongly indented or tabs are present:
 {{{#!diff
          boost::shared_ptr<memory::ZoneTableSegment> ztable_segment_;
          std::string name_;

 +        // cache_conf_ can be accessed only from this read-only getter,
 +9// to protect its integrity as much as possible.
          const internal::CacheConfig* getCacheConfig() const {
              return (cache_conf_.get());
 }}}

 {{{#!diff
 -    ZoneWriterPair getCachedZoneWriter(const dns::Name& zone);
 +    ZoneWriterPair getCachedZoneWriter(const dns::Name& zone,
 +9999       const std::string& datasrc_name = "");
 }}}

 If the segment (cache) is not writable now, will it ever become writable
 by auth server? I believe it'll be written by the zone manager and never
 by auth.
 {{{#!c++
         CACHE_NOT_WRITABLE, ///< The cache is not (yet) writable (and
 zones
                             ///  zones can't be loaded
 }}}

 This looks confused. The `DATASRC_NOT_FOUND` is not when the zone is
 missing in the named data source, but when the data source of the given
 name is not there.
 {{{#!c++
     /// that data source is placed.  Even if the given name of zone
 doesn't
     /// exist in the data source, other data sources are not searched and
     /// this method simply returns DATASRC_NOT_FOUND in the first element
     /// of the pair.
 }}}

 The docs for datasrc_name parameter is empty.

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