BIND 10 #1975: implement meta-or-container-of data source

[BIND 10 Development]

#1975: implement meta-or-container-of data source
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  vorner
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120612
  medium                             |            Resolution:
                  Component:  data   |             Sensitive:  0
  source                             |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  9
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 I forgot to comment on one other minor point and one other substantial
 point:

 The minor one: now we have two different classes that have
 "container".  It may be confusing, and we might want to rename one of
 them.  For example, `DataSourceClientContainer` might be renamed
 `DataSourceClientHolder`, or datasrc::Container might be renamed to
 `DataSource(Client?)List`.

 The substantial one: I'm concerned about the performance impact of
 `ConfigurableContainer::search`, especially due to the dynamic memory
 allocation and release, as this method is expected to be called in a
 performance sensitive path, i.e., auth query handling.
 It may not be that severe as new/delete for a small-size object should
 be relatively efficient, but I'd like to make it as efficient as
 possible from the beginning if we can do it by not making the code too
 ugly.  As commented, the reason why we use dynamic allocation is
 because `SearchResult` is not assignable.  I can think of a couple of
 ways to avoid that:
 - introduce a compatible assignable structure, only internally used in
   container.cc.  the search() method uses it inside it, replacing it
   as it finds a better result, and when it returns the final result,
   construct the public one from the internal structure.
 - introduce accessor methods to `SearchResult`, and maintain the
   corresponding member variables as non const.

 Likewise, I'd like to make this condition as lightweight as possible:
 {{{#!cpp
                     const uint8_t labels(
                         result.zone_finder->getOrigin().getLabelCount());
                     if (labels > candidate->matched_labels_) {
 }}}
 for performance sensitive setup, it's likely there's only one data
 source, and in many cases the search will result in a partial match.
 So it would be nicer if we can make the initial comparison simpler
 (besides, the current implementation assumes the default value of
 matched_labels_ (0) is always smaller than the result of a
 getLabelCount() call.  While it's true, relying on such an assumption
 seems to be less robust.)  I wonder we might change the code like
 this:
 {{{#!cpp
         case result::PARTIALMATCH:
             if (candiate.datasrc_ == NULL ||
                 candidate.matched_labels_ <
                 result.zone_finder->getOrigin().getLabelCount()) {
                 candiate = SearchResult(info.data_src_,
 result.zone_finder,
                                         labels, false);
             }
 }}}
 This will make the expression a bit more expensive for the second or
 higher iterations, but we'd consider such cases less performance
 sensitive.

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