BIND 10 #550: wildcard handling for memory zone: load

[BIND 10 Development]

#550: wildcard handling for memory zone: load
-------------------------------------+-------------------------------------
                 Reporter:  jinmei   |                Owner:  jinmei
                     Type:           |               Status:  reviewing
  enhancement                        |            Milestone:  A-Team-
                 Priority:  major    |  Sprint-20110209
                Component:  data     |           Resolution:
  source                             |            Sensitive:  0
                 Keywords:           |  Add Hours to Ticket:  0
Estimated Number of Hours:  5.0      |          Total Hours:  0
                Billable?:  1        |
                Internal?:  0        |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:5 vorner]:
 > Hello
 >
 > There are three minor nits:
 >  - There's an C-like artifact in addWildcards ‒ result is defined on the
 top and not used for a while. Is there an actual reason for that?

 Do you mean we should (be able to) do this

 {{{
 DomainTree::Result result = domains.insert(wname.split(1), &node);
 }}}

 instead of this?

 {{{
                 DomainTree::Result result;
 [comment lines]
                 result = domains.insert(wname.split(1), &node);
 }}}

 Hmm, I would normally prefer the former.  I don't remember exactly
 what I was thinking at that time, but I probably tried to keep the two
 sets of "insert() and assert()" consistent.  In this particular case
 that wouldn't be much different either way, because we don't do
 anything between the definition and the first use of result.  But in
 case we may add something between them it may be a bit better to defer
 the definition until we really need it (i.e., change it to the latter
 style).

 I don't have a strong opinion.  If you (even slightly) prefer the
 latter, I'll change it.

 >  - You changed the name to call to rrset->getName in addValidation. Not
 that I would oppose that, I don't really care, but I'm curious why.

 This was because we don't need a copy object of the name in
 addValidation() (unlike the add() itself, where we may modify the
 name), and passing rrset->getName() as a (const) reference seemed
 redundant (because it's part of another parameter, rrset), and because
 in this case getName() of the rrset should be very cheap (it's quite
 likely the rrset is actually a basic version of RRset, whose getName()
 simply returns a reference to its internal object).

 But it's not a strong opinion and can change if necessary or if it's
 deemed to be better.

 >  - The flags in the Node, do we expect that we will have so many flags
 that uint32_t will be needed? And how was 0x8000000U chosen?

 Good question.  I actually thought about it (and noted a bit about it
 in the documentation), but frankly I'm not entirely sure at this
 moment.  There are some specific possibilities in my mind though:
  - As documented, I expect we'll incorporate the node color to
    the flags (as a private 1-bit flag).
  - I also expect eventually we want to have a backpointer from a root
    of a sub tree (in the recursive tree organization of RBTree) to its
    parent tree and have a private "root" bit in the flags to identify
    the root nodes for performance optimizations.  (This is what BIND 9
    does also).
  - BIND 9 also uses some bit fields for NSEC processing.  I'm not sure
    we'll end up doing the same thing, but there may be possibilities.
  - BIND 9 also embeds node name related information such as the length
    of the name in a "flag" (in the form of bitfields).
  - Finally, we may want to store the node "ID" for faster name
    compression by exploiting the RBTree structure.  For that purpose
    we might want to have e.g. 4 real flags and a 28-bit wide field for
    the ID (to store up to about 260M nodes).

 That said, these all are just possibilities and may be premature at
 this stage.  So I don't mind beginning with a smaller width field.

 As for 0x8000000U, I was thinking we reserve higher several bits for
 application defined usage...hmm, it was a typo; it should have been
 0x80000000U in that sense.  In any case, even after the correction
 this is an ad hoc choice and may be changed in future versions.  So
 I'm okay with beginning with a more straightforward definition, such
 as 0x1 and 0x2.

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