-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
Add timestamp support to CompactedDBImpl #10030
Conversation
65ab62b
to
fca3bf4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jowlyzhang for the PR! Left some comments.
@riversand963 hello Yanqin, sending you this PR for a sanity check. The mini-crashtest failure seems legit because it constantly fails when I retry it, however I didn't quite get how the code path changed in this PR could be related to this crash because read only mode is disabled in this test: Do you have any suggestions for proceed to debug this crash? |
I don't think it's related to your change, so ignore it for now. More details should be #10032 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jowlyzhang for the PR
For the mini-crashtest failure, you may need to rebase onto latest main. |
3cbe257
to
e1ecdfb
Compare
@jowlyzhang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@jowlyzhang has updated the pull request. You must reimport the pull request before landing. |
@jowlyzhang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
…10047) Summary: After #10030 and #10004, we can enable checkpoint and backup in stress tests when user-defined timestamp is enabled. This PR has no production risk. Pull Request resolved: #10047 Test Plan: ``` TEST_TMPDIR=/dev/shm make crash_test_with_ts ``` Reviewed By: jowlyzhang Differential Revision: D36641565 Pulled By: riversand963 fbshipit-source-id: d86c9d87efcc34c32d1aa176af691d32b897644a
This PR is the second and last part for adding user defined timestamp support to read only DB. Specifically, the change in this PR includes:
options.timestamp
respected byCompactedDBImpl::Get
andCompactedDBImpl::MultiGet
to return results visible up till that timestamp.CompactedDBImpl::Get(...,std::string* timestsamp)
andCompactedDBImpl::MultiGet(std::vector<std::string>* timestamps)
return the timestamp(s) associated with the key(s).Test Plan: