Skip to content
This repository has been archived by the owner on Mar 9, 2019. It is now read-only.

Skip empty pages during cursor seek #452

Merged
merged 1 commit into from
Nov 9, 2015

Conversation

benbjohnson
Copy link
Member

Overview

This commit fixes an issue where keys are skipped by cursors after deletions occur in a bucket. This occurred because the cursor seeks to the leaf page but does not check if it is empty.

Fixes #429, #450


@lukechampine Thanks for digging into this and figuring it out. The fix you made makes sense. The cursor needs to move forward to the next item if an empty page is found. I made a small tweak to the fix inside next() so that it doesn't recursively call itself. I also added a test case to check for it. I added you as the commit author, though, for attribution.

Can you take a look and verify that this fix works for you in your project?

This commit fixes an issue where keys are skipped by cursors after
deletions occur in a bucket. This occurred because the cursor seeks
to the leaf page but does not check if it is empty.

Fixes boltdb#429, boltdb#450
@DavidVorick
Copy link

This PR passes all of our unit tests, which the master branch does not do. I'm doing a much more thorough test right now involving our pre-production system, usually takes a while but I'll report back when that finishes.

@lukechampine
Copy link
Contributor

Thanks for getting to the bottom of this, and for the attribution. Without a solid understanding of bolt's internals, my fixes were a shot in the dark. Since both First and Next use first internally, I was sure that first was where the real fix would need to be made. It was driving me a little crazy, haha.
I am surprised that this hasn't caused issues for anyone else. Maybe using ForEach after deletions is a rare use case. Does your new test case replicate the issue deterministically?
Also, just checking: does this affect any other Cursor operations, like Prev or Last?

@benbjohnson
Copy link
Member Author

@DavidVorick Awesome! let me know how the rest of the testing goes.

@lukechampine Your fix was pretty spot on. I think most people don't have a need to delete multiple items and then read them from a bucket in the same transaction. Many times mutation and data retrieval occur in separate transactions.

The test case seems to be deterministic. Writing 1000 keys ensures that it'll cover multiple pages and then deleting a large contiguous block at the beginning ensures that the first leaf page will be empty.

@DavidVorick
Copy link

@benbjohnson rest of testing has gone well. I appreciate you writing this, seems to have solved a frustrating bug

benbjohnson added a commit that referenced this pull request Nov 9, 2015
Skip empty pages during cursor seek
@benbjohnson benbjohnson merged commit d359c78 into boltdb:master Nov 9, 2015
@benbjohnson benbjohnson deleted the empty-seek branch November 9, 2015 16:36
@benbjohnson
Copy link
Member Author

@DavidVorick awesome, thanks for testing it out. Just merged. 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Boltdb Deleting Bucket?
3 participants