BIND 10 #2091: use encoded name data in RBNode

[BIND 10 Development]

#2091: use encoded name data in RBNode
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120731
  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:         |
  scalable inmemory                  |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:7 vorner]:

 Thanks for the review.

 > First of, the thing does not compile for me:

 Ah, okay, they were indeed redundant.  I hope I removed all offending
 ones.

 > Also, in documentation, should non-absolute be relative?

 I'm not sure exactly which documentation text you mean, and I don't
 necessarily think non-absolute is a bad term, but I updated one
 sentence to clarify that non absolute means relative.

 > However, I have most comments to the first two commits, with the memory
 segments.
 >
 > Because of the assert in the deallocator, I believe it is not possible
 to share a memory segment for multiple trees ‒ the one that'll be
 destroyed first will not have cleaned up all the memory, because some
 belongs to the other. Is it desired? If so, should it be at least
 documented?

 Not tested, but you're probably right, and that's not a longer term
 intention.  I clarified that as comments in ~ZoneData().

 > Also, the interface, where you pass the memory segment to many functions
 (insert, destroy), but it must be the same memory segment, that sounds
 like a very fragile interface. Would it be possible to somehow change
 that? I guess it is not the best thing to keep a reference inside, because
 then when it gets shared between applications, it is a problem. Or maybe
 it is not, since only one application would do the modifications and the
 others wouldn't need the reference.

 Yeah I know it's fragile, and I know it's better to keep a reference
 to the segment in the tree in that sense.  I didn't do it for the
 exact reason you guessed; we won't be able to store the segment object
 if it's in shared memory.  I don't like to rely too much on the
 possibility that only one process performs allocation/deallocation,
 and even with that assumption it may not work if it's from a
 memory-mapped file.

 I think a least bad approach is to introduce a separate
 accessor(modifier) that encapsulates the segment information and
 request applications to manipulate the zone data via the accessor.
 We'll eventually have something like `ZoneTableSegment` or
 `ZoneSegment` described in
 http://bind10.isc.org/wiki/ScalableZoneLoadDesign,
 so these could be naturally extended to support this feature.

 At the moment, I just added more detailed note about the issue to
 create().

 > And, what happens if you have two trees with different memory segments
 and swap them?

 It won't work.  But as we update this framework I guess it's less
 likely to be needed, at least as something compatible to the std::swap
 semantics.  In any case, at the moment I just added a note as comments
 that the memory segments must be the same for this to work.

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