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