Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

activate the AssertHeld() logic of Mutex and Spin classes. And address issue #100 #125

Merged
merged 2 commits into from
Mar 25, 2014

Conversation

matthewvon
Copy link
Contributor

Scary to find that Mutex.AssertHeld() test, used throughout the leveldb code, performs no test. This adds the necessary assert logic. PR is for post-Riak 2.0 code freeze.

I used this code on a really long Bulk test sequence. It did not find any mutex problems … thankfully.

This branch also contains an adjustment to VersionSet::LogAndApply(). Scott Fritchie created a test to randomly fail fail accesses. That test found a flaw in the LogAndApply() logic flow that was adjusted for 2.0. The logic flow now correctly differentiates between file failures and write failures … i.e. does not segfault.

MatthewVon added 2 commits February 28, 2014 09:26
…ns (error found by code in issue #100).  Populate Mutex.AssertHeld() code ... Google, why left empty?
@matthewvon matthewvon changed the title activate the AssertHeld() logic of Mutex and Spin classes. activate the AssertHeld() logic of Mutex and Spin classes. And address issue #100 Mar 20, 2014
@matthewvon matthewvon added this to the 2.0-beta milestone Mar 22, 2014
@andrewjstone
Copy link
Contributor

  • code inspection
  • make check

I couldn't get Scott's test in eleveldb to run, but it passed for him and Matthew.

👍

matthewvon pushed a commit that referenced this pull request Mar 25, 2014
activate the AssertHeld() logic of Mutex and Spin classes. And address issue #100
@matthewvon matthewvon merged commit d553b6c into develop Mar 25, 2014
@matthewvon matthewvon deleted the mv-mutex-assert branch August 19, 2015 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants