Skip to content

Commit

Permalink
Likely fix flaky TableFileCorruptedBeforeBackup (facebook#8151)
Browse files Browse the repository at this point in the history 
Summary:
Before corrupting a file in the DB and expecting corruption to
be detected, open DB read-only to ensure file is not made obsolete by
compaction. Also, to avoid obsolete files not yet deleted, only select
live files to corrupt.

Pull Request resolved: facebook#8151

Test Plan: watch CI

Reviewed By: akankshamahajan15

Differential Revision: D27568849

Pulled By: pdillinger

fbshipit-source-id: 39a69a2eafde0482b20a197949d24abe21952f27
  • Loading branch information
@pdillinger @facebook-github-bot
pdillinger authored and facebook-github-bot committed Apr 5, 2021
1 parent bd7ddf5 commit 96205ba
Showing 1 changed file with 26 additions and 13 deletions.
39 changes: 26 additions & 13 deletions utilities/backupable/backupable_db_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -658,14 +658,18 @@ class BackupableDBTest : public testing::Test {
return db;
}

void CloseAndReopenDB() {
void CloseAndReopenDB(bool read_only = false) {
// Close DB
db_.reset();

// Open DB
test_db_env_->SetLimitWrittenFiles(1000000);
DB* db;
ASSERT_OK(DB::Open(options_, dbname_, &db));
if (read_only) {
ASSERT_OK(DB::OpenForReadOnly(options_, dbname_, &db));
} else {
ASSERT_OK(DB::Open(options_, dbname_, &db));
}
db_.reset(db);
}

Expand Down Expand Up @@ -827,12 +831,19 @@ class BackupableDBTest : public testing::Test {

Status GetDataFilesInDB(const FileType& file_type,
std::vector<FileAttributes>* files) {
std::vector<std::string> live;
uint64_t ignore_manifest_size;
Status s = db_->GetLiveFiles(live, &ignore_manifest_size, /*flush*/ false);
if (!s.ok()) {
return s;
}
std::vector<FileAttributes> children;
Status s = test_db_env_->GetChildrenFileAttributes(dbname_, &children);
s = test_db_env_->GetChildrenFileAttributes(dbname_, &children);
for (const auto& child : children) {
FileType type;
uint64_t number = 0;
if (ParseFileName(child.name, &number, &type) && type == file_type) {
if (ParseFileName(child.name, &number, &type) && type == file_type &&
std::find(live.begin(), live.end(), "/" + child.name) != live.end()) {
files->push_back(child);
}
}
Expand Down Expand Up @@ -1469,7 +1480,7 @@ TEST_F(BackupableDBTest, TableFileCorruptedBeforeBackup) {
OpenDBAndBackupEngine(true /* destroy_old_data */, false /* dummy */,
kNoShare);
FillDB(db_.get(), 0, keys_iteration);
CloseAndReopenDB();
CloseAndReopenDB(/*read_only*/ true);
// corrupt a random table file in the DB directory
ASSERT_OK(CorruptRandomDataFileInDB(kTableFile));
// file_checksum_gen_factory is null, and thus table checksum is not
Expand All @@ -1485,7 +1496,7 @@ TEST_F(BackupableDBTest, TableFileCorruptedBeforeBackup) {
OpenDBAndBackupEngine(true /* destroy_old_data */, false /* dummy */,
kNoShare);
FillDB(db_.get(), 0, keys_iteration);
CloseAndReopenDB();
CloseAndReopenDB(/*read_only*/ true);
// corrupt a random table file in the DB directory
ASSERT_OK(CorruptRandomDataFileInDB(kTableFile));
// table file checksum is enabled so we should be able to detect any
Expand All @@ -1503,7 +1514,7 @@ TEST_F(BackupableDBTest, BlobFileCorruptedBeforeBackup) {
OpenDBAndBackupEngine(true /* destroy_old_data */, false /* dummy */,
kNoShare);
FillDB(db_.get(), 0, keys_iteration);
CloseAndReopenDB();
CloseAndReopenDB(/*read_only*/ true);
// corrupt a random blob file in the DB directory
ASSERT_OK(CorruptRandomDataFileInDB(kBlobFile));
// file_checksum_gen_factory is null, and thus blob checksum is not
Expand All @@ -1519,7 +1530,7 @@ TEST_F(BackupableDBTest, BlobFileCorruptedBeforeBackup) {
OpenDBAndBackupEngine(true /* destroy_old_data */, false /* dummy */,
kNoShare);
FillDB(db_.get(), 0, keys_iteration);
CloseAndReopenDB();
CloseAndReopenDB(/*read_only*/ true);
// corrupt a random blob file in the DB directory
ASSERT_OK(CorruptRandomDataFileInDB(kBlobFile));

Expand All @@ -1537,7 +1548,7 @@ TEST_P(BackupableDBTestWithParam, TableFileCorruptedBeforeBackup) {

OpenDBAndBackupEngine(true /* destroy_old_data */);
FillDB(db_.get(), 0, keys_iteration);
CloseAndReopenDB();
CloseAndReopenDB(/*read_only*/ true);
// corrupt a random table file in the DB directory
ASSERT_OK(CorruptRandomDataFileInDB(kTableFile));
// cannot detect corruption since DB manifest has no table checksums
Expand All @@ -1551,7 +1562,7 @@ TEST_P(BackupableDBTestWithParam, TableFileCorruptedBeforeBackup) {
options_.file_checksum_gen_factory = GetFileChecksumGenCrc32cFactory();
OpenDBAndBackupEngine(true /* destroy_old_data */);
FillDB(db_.get(), 0, keys_iteration);
CloseAndReopenDB();
CloseAndReopenDB(/*read_only*/ true);
// corrupt a random table file in the DB directory
ASSERT_OK(CorruptRandomDataFileInDB(kTableFile));
// corruption is detected
Expand All @@ -1567,7 +1578,7 @@ TEST_P(BackupableDBTestWithParam, BlobFileCorruptedBeforeBackup) {
options_.enable_blob_files = true;
OpenDBAndBackupEngine(true /* destroy_old_data */);
FillDB(db_.get(), 0, keys_iteration);
CloseAndReopenDB();
CloseAndReopenDB(/*read_only*/ true);
// corrupt a random blob file in the DB directory
ASSERT_OK(CorruptRandomDataFileInDB(kBlobFile));
// cannot detect corruption since DB manifest has no blob file checksums
Expand All @@ -1581,7 +1592,7 @@ TEST_P(BackupableDBTestWithParam, BlobFileCorruptedBeforeBackup) {
options_.file_checksum_gen_factory = GetFileChecksumGenCrc32cFactory();
OpenDBAndBackupEngine(true /* destroy_old_data */);
FillDB(db_.get(), 0, keys_iteration);
CloseAndReopenDB();
CloseAndReopenDB(/*read_only*/ true);
// corrupt a random blob file in the DB directory
ASSERT_OK(CorruptRandomDataFileInDB(kBlobFile));
// corruption is detected
Expand Down Expand Up @@ -2050,14 +2061,16 @@ TEST_F(BackupableDBTest, TableFileCorruptionBeforeIncremental) {
ASSERT_OK(dbi->Put(WriteOptions(), "y", Random(42).RandomString(500)));
ASSERT_OK(dbi->Flush(FlushOptions()));
ASSERT_OK(dbi->TEST_WaitForFlushMemTable());
CloseDBAndBackupEngine();
CloseAndReopenDB(/*read_only*/ true);

std::vector<FileAttributes> table_files;
ASSERT_OK(GetDataFilesInDB(kTableFile, &table_files));
ASSERT_EQ(table_files.size(), 2);
std::string tf0 = dbname_ + "/" + table_files[0].name;
std::string tf1 = dbname_ + "/" + table_files[1].name;

CloseDBAndBackupEngine();

if (corrupt_before_first_backup) {
// This corrupts a data block, which does not cause DB open
// failure, only failure on accessing the block.
Expand Down

0 comments on commit 96205ba

Please sign in to comment.