-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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 support for BlobDB to ldb #9630
Conversation
8b0b6fa
to
dfba858
Compare
@changneng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
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 a lot for the PR @changneng ! Please see some comments below.
@changneng has updated the pull request. You must reimport the pull request before landing. |
3 similar comments
@changneng has updated the pull request. You must reimport the pull request before landing. |
@changneng has updated the pull request. You must reimport the pull request before landing. |
@changneng has updated the pull request. You must reimport the pull request before landing. |
a58b917
to
eab2ba5
Compare
@changneng has updated the pull request. You must reimport the pull request before landing. |
@changneng has updated the pull request. You must reimport the pull request before landing. |
@changneng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
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 for the updates @changneng ! The changes look good in general 👍👍; just some minor comments
@changneng One more thing - could you please mention this change in |
@changneng has updated the pull request. You must reimport the pull request before landing. |
1 similar comment
@changneng has updated the pull request. You must reimport the pull request before landing. |
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.
LGTM! Thanks for contributing to RocksDB @changneng !
@changneng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
…st cases for BlobDB options in `ldb`
2. Remove `blobParams` in `dump` the test case 3. Validation: `min_blob_size >= 0` instead of `min_blob_size > 0` 4. Use `auto` to define iterator for convenience
fd7ca43
to
0bd2f33
Compare
@changneng has updated the pull request. You must reimport the pull request before landing. |
@changneng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@changneng has updated the pull request. You must reimport the pull request before landing. |
@changneng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary:
Add the configuration options and help messages of BlobDB to
ldb
Test Plan:
python ./tools/ldb_test.py