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

Reset DBIter::saved_key_ with proper user key anywhere before pass to DBIter::FindNextUserEntry #1962

Closed
wants to merge 5 commits into from

Conversation

lightmark
Copy link
Contributor

fix db_iter bug introduced by facebook#1413

@facebook-github-bot
Copy link
Contributor

@lightmark has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

db/db_iter.cc Outdated
@@ -961,6 +960,7 @@ void DBIter::Seek(const Slice& target) {
}
direction_ = kForward;
ClearSavedValue();
saved_key_.Reserve(saved_key_.Size() - 8);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add more comments explaining when we are using InternalKey and when we are using UserKey and why is that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I will make a new PR to refactor IterKey soon

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a simple comment saying that this line purpose is to convert the internal key to a user key because FindNextUserEntry needs a UserKey to be stored in saved_key_

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Added

@facebook-github-bot
Copy link
Contributor

@lightmark updated the pull request - view changes - changes since last import

@facebook-github-bot
Copy link
Contributor

@lightmark updated the pull request - view changes - changes since last import

@lightmark lightmark changed the title Truncate save_key_ to user key before passed to DBIter::FindNextUserEntry reset save_key_ with proper user key anywhere before passed to DBIter::FindNextUserEntry Mar 8, 2017
@lightmark lightmark changed the title reset save_key_ with proper user key anywhere before passed to DBIter::FindNextUserEntry reset save_key_ with proper user key anywhere before pass to DBIter::FindNextUserEntry Mar 8, 2017
@lightmark lightmark changed the title reset save_key_ with proper user key anywhere before pass to DBIter::FindNextUserEntry Reset DBIter::saved_key_ with proper user key anywhere before pass to DBIter::FindNextUserEntry Mar 8, 2017
@facebook-github-bot
Copy link
Contributor

@lightmark has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@lightmark updated the pull request - view changes - changes since last import

Copy link
Contributor

@IslamAbdelRahman IslamAbdelRahman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I still don't see the comment :D

@facebook-github-bot
Copy link
Contributor

@lightmark updated the pull request - view changes - changes since last import

@facebook-github-bot
Copy link
Contributor

@lightmark has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

IslamAbdelRahman pushed a commit that referenced this pull request Mar 9, 2017
… DBIter::FindNextUserEntry

Summary:
fix db_iter bug introduced by [#1413](#1413)
Closes #1962

Differential Revision: D4672369

Pulled By: lightmark

fbshipit-source-id: 6a22953
lightmark added a commit that referenced this pull request Mar 14, 2017
… DBIter::FindNextUserEntry

Summary:
fix db_iter bug introduced by [#1413](#1413)
Closes #1962

Differential Revision: D4672369

Pulled By: lightmark

fbshipit-source-id: 6a22953
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.

3 participants