andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix flakiness of BackupableDBTest.TableFileCorruptedBeforeBackup (#7082)

Browse Source 
Summary:
If the corruption of a table file is done before flushing, then db manifest may record the checksum for the corrupted table, which results in "matching checksums" when backup engine tries to verfiy the checksum, and causes a flaky test.

Fix the issue by adding `Flush()` before trying to corrupt a table file in *db*.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/7082

Test Plan:
`buck test`
Without the fix, failed 5 of 100 tests.
Suspected whether the pseudo randomness causes the issue: doubling `keys_iteration` resulted in 2 of 100 tests failed; deterministically corrupting tables file also caused 2 of 100 tests to fail.
With the fix, passed 200 of 200 tests.

Reviewed By: pdillinger

Differential Revision: D22375421

Pulled By: gg814

fbshipit-source-id: 7304618e7520684b6087e42d0b58329c5ad18329
This commit is contained in:
Zitan Chen 2020-07-03 15:38:35 -07:00 committed by Facebook GitHub Bot
parent ca7659e2c4
commit 147f7b472a
2 changed files with 5 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
utilities/backupable/backupable_db.cc
View File

@ -1586,7 +1586,7 @@ Status BackupEngineImpl::AddBackupFileWorkItem(
// b) share_table_files is true and the file exists already.
if (kDbFileChecksumFuncName == src_checksum_func_name) {
if (src_checksum_str == kUnknownFileChecksum) {
return Status::Aborted("Unkown checksum value for " + fname);
return Status::Aborted("Unknown checksum value for " + fname);
}
s = CalculateChecksum(src_dir + fname, db_env_, src_env_options, size_limit,
&checksum_value);

4
utilities/backupable/backupable_db_test.cc
View File

@ -1185,6 +1185,7 @@ TEST_F(BackupableDBTest, TableFileCorruptedBeforeBackup) {
OpenDBAndBackupEngine(true /* destroy_old_data */, false /* dummy */,
kNoShare);
FillDB(db_.get(), 0, keys_iteration);
ASSERT_OK(db_->Flush(FlushOptions()));
// corrupt a random table file in the DB directory
ASSERT_OK(CorruptRandomTableFileInDB());
// file_checksum_gen_factory is null, and thus table checksum is not
@ -1199,6 +1200,7 @@ TEST_F(BackupableDBTest, TableFileCorruptedBeforeBackup) {
OpenDBAndBackupEngine(true /* destroy_old_data */, false /* dummy */,
kNoShare);
FillDB(db_.get(), 0, keys_iteration);
ASSERT_OK(db_->Flush(FlushOptions()));
// corrupt a random table file in the DB directory
ASSERT_OK(CorruptRandomTableFileInDB());
// table file checksum is enabled so we should be able to detect any
@ -1216,6 +1218,7 @@ TEST_P(BackupableDBTestWithParam, TableFileCorruptedBeforeBackup) {
OpenDBAndBackupEngine(true /* destroy_old_data */);
FillDB(db_.get(), 0, keys_iteration);
ASSERT_OK(db_->Flush(FlushOptions()));
// corrupt a random table file in the DB directory
ASSERT_OK(CorruptRandomTableFileInDB());
// cannot detect corruption since DB manifest has no table checksums
@ -1228,6 +1231,7 @@ TEST_P(BackupableDBTestWithParam, TableFileCorruptedBeforeBackup) {
options_.file_checksum_gen_factory = GetFileChecksumGenCrc32cFactory();
OpenDBAndBackupEngine(true /* destroy_old_data */);
FillDB(db_.get(), 0, keys_iteration);
ASSERT_OK(db_->Flush(FlushOptions()));
// corrupt a random table file in the DB directory
ASSERT_OK(CorruptRandomTableFileInDB());
// corruption is detected