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

WT-3144 bug fix: random cursor returns not-found when descending to an empty page #3289

Merged
merged 7 commits into from Feb 14, 2017

Conversation

@keithbostic
Copy link
Member

keithbostic commented Feb 10, 2017

If random descent through the tree fails, fallback to skipping through the tree's pages; if skipping through the tree's pages fails, fallback to a random entry from the first page in the tree that contains anything at all.

keithbostic added 3 commits Feb 10, 2017
…n empty page

Fix a documentation error, we never implemented a next_random_sample_percent
configuration.
the tree's pages; if skipping through the tree's pages fails, fallback
to a random entry from the first page in the tree that contains anything
at all.
* is really empty, fallback to skipping through tree pages.
*/
if (ret == WT_NOTFOUND)
goto fallback_walk;

This comment has been minimized.

Copy link
@michaelcahill

michaelcahill Feb 12, 2017

Member

As far as I can see, we won't have done the calculation of cbt->next_random_leaf_skip, so it looks like we'll always find the first record in the tree. Am I missing something?

This comment has been minimized.

Copy link
@keithbostic

keithbostic Feb 13, 2017

Author Member

we won't have done the calculation of cbt->next_random_leaf_skip, so it looks like we'll always find the first record in the tree.

Agreed, I've done another run at this one, ready for your review.

for i in range(1,5):
self.assertTrue(cursor.next(), wiredtiger.WT_NOTFOUND)
cursor.close

This comment has been minimized.

Copy link
@michaelcahill

michaelcahill Feb 12, 2017

Member

It looks like we should have a test that creates a tree with enough data for multiple pages, reopens the connection so we have a real tree, then truncates most / all of the tree and makes sure random lookups find data / fail (respectively). That way we're testing WT_REF_DELETED, not just empty pages.

This comment has been minimized.

Copy link
@keithbostic

keithbostic Feb 13, 2017

Author Member

It looks like we should have a test that creates a tree with enough data for multiple pages, reopens the connection so we have a real tree, then truncates most / all of the tree and makes sure random lookups find data / fail (respectively). That way we're testing WT_REF_DELETED, not just empty pages.

Added in fc30d0e.

keithbostic added 4 commits Feb 13, 2017
…pens the

connection so we have a real tree, then truncates most / all of the tree and
makes sure random lookups find data / fail (respectively). That way we're
testing WT_REF_DELETED, not just empty pages.
The bug being fixed is that WT_CURSOR.next_random_leaf_skip wasn't set
when random descent failed and we were falling back to skipping through
pages, but restructuring the code made things simpler.

Turn off cursor ordering checks: if the tree is sufficiently sparse,
we can end up calling WT_CURSOR.next multiple times, and failing those
checks.
@michaelcahill

This comment has been minimized.

Copy link
Member

michaelcahill commented Feb 14, 2017

Thanks @keithbostic, lgtm. I'll merge.

@michaelcahill michaelcahill merged commit 988c297 into develop Feb 14, 2017
1 check passed
1 check passed
default Build finished.
Details
@michaelcahill michaelcahill deleted the wt-3144-random-cursor-notfound branch Feb 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.