-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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
Augment sst_dump tool to verify num_entries in table property #12322
Conversation
@cbi42 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.
Thank you for this improvement! LGTM.
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@@ -55,6 +55,8 @@ void print_help(bool to_stderr) { | |||
|
|||
--command=check|scan|raw|verify|identify | |||
check: Iterate over entries in files but don't print anything except if an error is encountered (default command) | |||
When read_num, from and to are not set, it compares the number of keys read with num_entries in table |
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.
read_num -> read_num_limit
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.
This part is used in the CLI so I did not include the change in this PR. We can change them separately.
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.
Thank you! Left one nit comment for fixing comment :)
Summary: Add `SstFileReader::VerifyNumEntries()` for this purpose. I added the same functionality to `sst_dump` in #12322. Since sst_file_reader.h is exposed to users while sst_dump.h is not, it seems more appropriate to add SST files related APIs here. Pull Request resolved: #12418 Test Plan: `./sst_file_reader_test --gtest_filter="*VerifyNumEntries*"` Reviewed By: jowlyzhang Differential Revision: D54764271 Pulled By: cbi42 fbshipit-source-id: 22ebfe04bbb0b152762cee13d4210b147b36d3e9
…k#12418) Summary: Add `SstFileReader::VerifyNumEntries()` for this purpose. I added the same functionality to `sst_dump` in facebook#12322. Since sst_file_reader.h is exposed to users while sst_dump.h is not, it seems more appropriate to add SST files related APIs here. Pull Request resolved: facebook#12418 Test Plan: `./sst_file_reader_test --gtest_filter="*VerifyNumEntries*"` Reviewed By: jowlyzhang Differential Revision: D54764271 Pulled By: cbi42 fbshipit-source-id: 22ebfe04bbb0b152762cee13d4210b147b36d3e9
Summary: sst_dump --command=check can now compare number of keys in a file with num_entries in table property and reports corruption is there is a mismatch.
Test plan:
SstFileDumper::ReadSequential