Improve ldb consistency checks #6802
Conversation
There was a problem hiding this comment.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@siying has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
tools/ldb_tool.cc
Outdated
| @@ -46,6 +46,8 @@ void LDBCommandRunner::PrintHelp(const LDBOptions& ldb_options, | |||
| " : DB supports ttl and value is internally timestamp-suffixed\n"); | |||
| ret.append(" --" + LDBCommand::ARG_TRY_LOAD_OPTIONS + | |||
| " : Try to load option file from DB.\n"); | |||
| ret.append(" --" + LDBCommand::ARG_DISABLE_CONSISTENCY_CHECKS + | |||
| " : Set options.disable_consistency_checks = false.\n"); | |||
There was a problem hiding this comment.
force_consistency_checks
tools/ldb_cmd_test.cc
Outdated
| ASSERT_OK(db->Put(wopts, "bar2", "4")); | ||
| ASSERT_OK(db->Flush(fopts)); | ||
|
|
||
| delete db; |
There was a problem hiding this comment.
recommend db = nullptr or wrap in curly braces
tools/ldb_cmd.cc
Outdated
| @@ -362,6 +364,9 @@ LDBCommand::LDBCommand(const std::map<std::string, std::string>& options, | |||
| is_db_ttl_ = IsFlagPresent(flags, ARG_TTL); | |||
| timestamp_ = IsFlagPresent(flags, ARG_TIMESTAMP); | |||
| try_load_options_ = IsFlagPresent(flags, ARG_TRY_LOAD_OPTIONS); | |||
| force_consistency_checks_ = | |||
| !IsFlagPresent(flags, ARG_DISABLE_CONSISTENCY_CHECKS); | |||
| fprintf(stderr, "%ld\n", (long)force_consistency_checks_); | |||
tools/ldb_cmd.cc
Outdated
| @@ -622,6 +628,8 @@ Options LDBCommand::PrepareOptionsForOpenDB() { | |||
| } | |||
| } | |||
|
|
|||
| fprintf(stderr, "%ld\n", (long)force_consistency_checks_); | |||
|
I did try something like "--enable_consistency_checks". The problem is that, the command line doesn't allow something like --enable_consistency_checks=0. So we have to make not setting default and setting optional. But I want to make enabling consistency check default. Then I have to make the negative turn as command line. I don't have a better choice. |
|
@siying has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@siying has updated the pull request. Re-import the pull request |
|
@siying has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
───────────────────████
───────────────────█████
───────────────────██████
───────────────────███████
──────────────────████████
──────────────────████████
─────────────────█████████
────────────────█████████
───────────────█████████
──────────────█████████
──────────────██████████████████
────────────█████████████████████
───────────███████████████████████
████████─██████████████████████████
████████─███████████████████████████
████████─████████████████████████████
████████─████████████████████████████
████████─████████████████████████████
████████─████████████████████████████
████████─███████████████████████████
████████─██████████████████████████
████████─█████████████████████████
████████─████████████████████████
████████─███████████████████████
|
@siying has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: When using ldb, users cannot turn on force consistency check in most commands, while they cannot use checksonsistnecy with --try_load_options. The change fixes both by: 1. checkconsistency now calls OpenDB() so that it gets all the options loading and sanitized options logic 2. use options.check_consistency_checks = true by default, and add a --disable_consistency_checks to turn it off. Test Plan: Add a new unit test. Some manual tests with corrupted DBs.
|
@siying has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@siying has updated the pull request. Re-import the pull request |
|
@siying has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
This pull request has been merged in a50ea71. |
Summary: When using ldb, users cannot turn on force consistency check in most commands, while they cannot use checksonsistnecy with --try_load_options. The change fixes both by: 1. checkconsistency now calls OpenDB() so that it gets all the options loading and sanitized options logic 2. use options.check_consistency_checks = true by default, and add a --disable_consistency_checks to turn it off. Pull Request resolved: #6802 Test Plan: Add a new unit test. Some manual tests with corrupted DBs. Reviewed By: pdillinger Differential Revision: D21388051 fbshipit-source-id: 8d122732d391b426e3982a1c3232a8e3763ffad0
Summary:
When using ldb, users cannot turn on force consistency check in most commands, while they cannot use checksonsistnecy with --try_load_options. The change fixes both by:
Test Plan: Add a new unit test. Some manual tests with corrupted DBs.