BIND 10 #192: Data source hotspot cache

[BIND 10 Development]

#192: Data source hotspot cache
-------------------------+--------------------------------------------------
 Reporter:  each         |        Owner:  jinmei                                        
     Type:  enhancement  |       Status:  reviewing                                     
 Priority:  major        |    Milestone:  05. 3rd Incremental Release: Serious Secondary
Component:  b10-auth     |   Resolution:                                                
 Keywords:               |    Sensitive:  0                                             
-------------------------+--------------------------------------------------

Comment(by jinmei):

 Follow up comments on the Name/DataSrcMatch desgin.  I'm okay with the
 resulting code, but will respond to your points:

 > > Since !NameMatch is a public class that may be used in other context
 than processing an incoming query, the added mandatory parameter and the
 additional embeeded logic make unnecessarily limit the applicability of
 the class.
 >
 > What other context?  It currently returns the best enclosing zone match
 and the best data source; I really can't think of a purpose for this other
 than "figure out which zone and data source is authoritative for this
 name".  (Remember, the whole original purpose was to avoid the need for a
 Name() constructor.)

 That's actually my point.  And, for the purpose of "figure out which zone
 and data source is authoritative for this name", we don't need RRtype.

 As for "what other context", I can think of various cases, but as you
 yourself noted (in the next paragraph) dynamic update is one possibility.
 XFR is another.

 You'd perhaps say we can pass a special RRType value such as ANY to
 indicate it's type independent, but adding the additional argument, which
 also requires additional test cases as we saw (and found it unnecessary in
 the end), just for a very special case of DS query doesn't make sense to
 me at all.

 The additional dependency for a niche purpose is itself a problem in
 general.  Consider, for example, the name to be matched may differ for
 some specific type of network in some very limited cases, and we add an
 argument of asio::io_service to the Name/DataSrcMatch constructor for the
 convenient of that specifi case.  In such an extreme case I guess we can
 all agree that it's overkilling.

 Admittedly, this is a hypothetical example, and in this specific case
 importing RRType in addition to RRClass wouldn't be so different from
 importing just RRClass.  However, in the generic sense of dependency
 concerns, IMO we should keep dependency as small as possible unless
 there's a strong reason to do so (instead of "it may be useful in some
 cases").

 Looking at this issue from this point of view, while we *could* pass
 RRType to Name/DataSrcMatch and have it tweak the name depending on the
 type, we *do not have to*: the only caller of this class for now is in
 data_source.cc, so it's just a matter of where we do this.  If we do it in
 Name/DataSrcMatch, we increase dependency in a public header file; if we
 do it in data_source.cc, we can hide it from other applications.  In the
 sense of generic dependency consideration, the choice is obvious to me.

 If, in the future, it turns out we need to tweak the name based on some
 RRType in so many places, we can then consider extending the class so that
 it will also accept an RRtype argument and adjust the behavior depending
 on the type.  And, this can be a backward compatible change.  Until then,
 we should keep public classes as small as possible.

 > That said, I'm fine with the separation you suggested (for the reason
 below).  But I don't see why !DataSrcMatch needs to be kept general-
 purpose, because I can't think of anything *other* than query or update
 processing that would ever need to find a zone and data source.

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