Skip to content

Fix extra tombstones on update #197

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

Merged
merged 2 commits into from
Sep 17, 2014
Merged

Conversation

engelsanchez
Copy link
Contributor

An unrelated fix in commit 4ab96d4 caused updates replacing a value
in the same file to write extra tombstones that serve no purpose.
Updates are supposed to create a tombstone for previous values only
when the previous value was stored in an older file. Multiple
values for the same key in the same file do not require these extra
tombstones.

This PR demonstrates the problem with a new test in the first commit.
The second commit contains the fix to the update logic.

This addresses issue #196

Our current delete scheme requires that updates write tombstones for
previous values when the previous value is in an file logically older
than the current write file.  An unrelated change broke that logic, and
now tombstones are written every single time a value is updated.

This test demonstrates this by counting the number of tombstones
expected for repeatedly updating the same key when two data files are
created. Only a tombstone is required, but we get one per update.
An unrelated fix in commit 4ab96d4 caused updates replacing a value
in the same file to write extra tombstones that serve no purpose.
Updates are supposed to create a tombstone for previous values only
when the previous value was stored in an older file. Multiple
values for the same key in the same file do not require these extra
tombstones.
@engelsanchez engelsanchez added this to the 2.0.1 milestone Sep 15, 2014
@slfritchie
Copy link
Contributor

+1 a7b0c1d

Review, 8 hours of QuickCheck + PULSE runs all look good

borshop added a commit that referenced this pull request Sep 17, 2014
Fix extra tombstones on update

Reviewed-by: slfritchie
@slfritchie
Copy link
Contributor

@borshop merge

@borshop borshop merged commit a7b0c1d into 1.7 Sep 17, 2014
@seancribbs seancribbs deleted the bugfix/extra-tombstones-on-update branch April 1, 2015 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants