BIND 10 #2831: define and implement MemorySegmentMapped

BIND 10 Development do-not-reply at isc.org
Mon Apr 29 20:42:37 UTC 2013


#2831: define and implement MemorySegmentMapped
-------------------------------------+-------------------------------------
            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:  5             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  shared memory data source
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:19 muks]:

 > > Okay, I've tried to implement it.  It resulted in non trivial changes
 > > to the documentation (it now more clearly says reader and writer or
 > > multi writers can't be mixed).
 >
 > About the comments regarding the possibility of a race in `Impl()`,
 > would creating a `BaseSegment` on a temporary file and then `rename()`
 > it into place avoid the race?

 I doubt it...in this case I suspect the creator process acquires a
 lock before renaming and locks acquired after that (on the newly
 created file) won't protect the same data.  Even if it prevents this
 small chance of race, I don't think it worth the additional complexity
 of the code.  After all, the lock is used as an additional layer of
 defense and it's primarily the responsibility of the user.

 > > > * Aside from minor comment updates, I have pushed some code fixes to
 the
 > > >   branch. Please check these.
 > >
 > > These are generally look okay, but I reverted or further updated some
 > > of them:
 > >
 > > 8297b3a [2831] reverted the change for deleting checkNamedData().
 > > 6eae357 [2831] removed test for segment size after shrink.
 >
 > * Regarding commit `6eae3579a0dc0ed27f33c3e36cec2ad64be7329d`, I wanted
 >   to see a testcase where the segment is asserted to shrink and the data
 >   in it is tested to be consistent. If we can't do it this way, we
 >   should try to test it some other way.

 As we discussed for the "shrink" test, we cannot be 100% sure the
 segment size really becomes smaller after shrinkToFit(), so we simply
 cannot assert this condition:
 {{{#!diff
 -    EXPECT_GT(old_size, segment_->getSize());
 }}}
 But we at least test the data integrity, and, in practice, it's quite
 likely to be the case that the size actually decreases, so I think the
 current test is good enough for this purpose.

 > > > * This branch generates a Valgrind failure:
 > > > {{{
 > > > [ RUN      ] MemorySegmentMappedTest.namedAddress
 > > > ==17022== Syscall param msync(start) points to uninitialised byte(s)
 > > > ==17022==    at 0x371C00E7F0: __msync_nocancel (syscall-
 template.S:81)
 > > > ==17022==    by 0x4C2CB3F:
 boost::interprocess::basic_managed_mapped_file<char,
 boost::interprocess::rbtree_best_fit<boost::interprocess::null_mutex_family,
 boost::interprocess::offset_ptr<void, long, unsigned long, 0ul>, 0ul>,
 boost::interprocess::iset_index>::flush() (mapped_region.hpp:540)
 > > > [       OK ] MemorySegmentMappedTest.namedAddress (84 ms)
 [...]
 > It looks like `find_or_construct()` is causing this.

 To be sure, is it okay to leave it open then?

 > I have pushed another minor comment update commit to this branch.

 It looks fine.

 > Other than the minor task about a testcase for shrink discussed above,
 > this branch looks good to me now, so you can go ahead and merge it.

 I've not updated the branch for now as explained above.  Please let me
 know if the current version is okay for merge or you still have
 something to be resolved.

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


More information about the bind10-tickets mailing list