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

Support WBWI for keys having timestamps #9603

Closed
wants to merge 1 commit into from

Conversation

riversand963
Copy link
Contributor

@riversand963 riversand963 commented Feb 18, 2022

This PR supports inserting keys to a WriteBatchWithIndex for column families that enable user-defined timestamps
and reading the keys back. The index does not have timestamps.

Writing a key to WBWI is unchanged, because the underlying WriteBatch already supports it.
When reading the keys back, we need to make sure to distinguish between keys with and without timestamps before
comparison.

When user calls GetFromBatchAndDB(), no timestamp is needed to query the batch, but a timestamp has to be
provided to query the db. The assumption is that data in the batch must be newer than data from the db.

Test plan:
make check

@riversand963 riversand963 changed the title Support WBWI indexing WriteBatch with timestamps Support WBWI for keys having timestamps Feb 18, 2022
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

@riversand963 has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D34354849

Copy link
Contributor

@ltamasi ltamasi left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @riversand963 ! LGTM in general, just a few minor comments/questions

db/write_batch_test.cc Outdated Show resolved Hide resolved
Comment on lines +1016 to +1017
ASSERT_OK(wb1.Put(&cf0, "key", "value"));
ASSERT_FALSE(WriteBatchInternal::HasKeyWithTimestamp(wb1));
Copy link
Contributor

Choose a reason for hiding this comment

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

For the sake of completeness, we could consider adding coverage for the timestamp-less Delete and SingleDelete too.

Summary:
This PR supports inserting keys to a `WriteBatchWithIndex` for column families that enable user-defined timestamps
and reading the keys back. **The index does not have timestamps.**

Writing a key to WBWI is unchanged, because the underlying WriteBatch already supports it.
When reading the keys back, we need to make sure to distinguish between keys with and without timestamps before
comparison.

When user calls `GetFromBatchAndDB()`, no timestamp is needed to query the batch, but a timestamp has to be
provided to query the db. The assumption is that data in the batch must be newer than data from the db.

Pull Request resolved: facebook#9603

Test Plan: make check

Differential Revision: D34354849

Pulled By: riversand963

fbshipit-source-id: 8721eb10de5b3abaca7259f8dc6a54785706e35a
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D34354849

@riversand963
Copy link
Contributor Author

Thanks @ltamasi for the review!

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.

None yet

3 participants