Fix flaky test BackupableDBTest.FileSizeForIncremental (#8197)

Summary:
Test was flaky because for kUseDbSessionId naming, blob files use
naming scheme kLegacyCrc32cAndFileSize. So expected number of files
because of collision can vary. So disabling blobdb for this test case.

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

Reviewed By: pdillinger

Differential Revision: D27836997

Pulled By: akankshamahajan15

fbshipit-source-id: 5eb21a5f4acae3d6b730a9e1b207264fbc18cb80
main
Akanksha Mahajan 4 years ago committed by Facebook GitHub Bot
parent 531a5f88a1
commit c377c2ba15
  1. 16
      utilities/backupable/backupable_db_test.cc

@ -2168,6 +2168,8 @@ TEST_F(BackupableDBTest, TableFileCorruptionBeforeIncremental) {
// between incremental backups // between incremental backups
TEST_F(BackupableDBTest, FileSizeForIncremental) { TEST_F(BackupableDBTest, FileSizeForIncremental) {
const auto share_no_checksum = static_cast<ShareFilesNaming>(0); const auto share_no_checksum = static_cast<ShareFilesNaming>(0);
// TODO: enable blob files once Integrated BlobDB supports DB session id.
options_.enable_blob_files = false;
for (ShareFilesNaming option : {share_no_checksum, kLegacyCrc32cAndFileSize, for (ShareFilesNaming option : {share_no_checksum, kLegacyCrc32cAndFileSize,
kNamingDefault, kUseDbSessionId}) { kNamingDefault, kUseDbSessionId}) {
@ -2192,9 +2194,9 @@ TEST_F(BackupableDBTest, FileSizeForIncremental) {
ASSERT_OK(backup_engine_->CreateNewBackup(db_.get(), true /*flush*/)); ASSERT_OK(backup_engine_->CreateNewBackup(db_.get(), true /*flush*/));
CloseDBAndBackupEngine(); CloseDBAndBackupEngine();
// Corrupt backup SST and blob file // Corrupt backup SST file
ASSERT_OK(file_manager_->GetChildrenFileAttributes(shared_dir, &children)); ASSERT_OK(file_manager_->GetChildrenFileAttributes(shared_dir, &children));
ASSERT_EQ(children.size(), 2U); // one sst and one blob file ASSERT_EQ(children.size(), 1U); // one sst
for (const auto& child : children) { for (const auto& child : children) {
if (child.name.size() > 4 && child.size_bytes > 0) { if (child.name.size() > 4 && child.size_bytes > 0) {
ASSERT_OK( ASSERT_OK(
@ -2251,10 +2253,10 @@ TEST_F(BackupableDBTest, FileSizeForIncremental) {
OpenDBAndBackupEngine(false, false, share); OpenDBAndBackupEngine(false, false, share);
ASSERT_OK(db_->Put(WriteOptions(), "y", Random(42).RandomString(500))); ASSERT_OK(db_->Put(WriteOptions(), "y", Random(42).RandomString(500)));
// Count backup SSTs and blob files. // Count backup SSTs files.
children.clear(); children.clear();
ASSERT_OK(file_manager_->GetChildrenFileAttributes(shared_dir, &children)); ASSERT_OK(file_manager_->GetChildrenFileAttributes(shared_dir, &children));
ASSERT_EQ(children.size(), 4U); // two sst and two blob files ASSERT_EQ(children.size(), 2U); // two sst files
// Try create backup 3 // Try create backup 3
s = backup_engine_->CreateNewBackup(db_.get(), true /*flush*/); s = backup_engine_->CreateNewBackup(db_.get(), true /*flush*/);
@ -2267,18 +2269,18 @@ TEST_F(BackupableDBTest, FileSizeForIncremental) {
// Acceptable to call it corruption if size is not in name and // Acceptable to call it corruption if size is not in name and
// db session id collision is practically impossible. // db session id collision is practically impossible.
EXPECT_TRUE(s.IsCorruption()); EXPECT_TRUE(s.IsCorruption());
EXPECT_EQ(children.size(), 4U); // no SST/Blob file added EXPECT_EQ(children.size(), 2U); // no SST file added
} else if (option == share_no_checksum) { } else if (option == share_no_checksum) {
// Good to call it corruption if both backups cannot be // Good to call it corruption if both backups cannot be
// accommodated. // accommodated.
EXPECT_TRUE(s.IsCorruption()); EXPECT_TRUE(s.IsCorruption());
EXPECT_EQ(children.size(), 4U); // no SST/Blob file added EXPECT_EQ(children.size(), 2U); // no SST file added
} else { } else {
// Since opening a DB seems sufficient for detecting size corruption // Since opening a DB seems sufficient for detecting size corruption
// on the DB side, this should be a good thing, ... // on the DB side, this should be a good thing, ...
EXPECT_OK(s); EXPECT_OK(s);
// ... as long as we did actually treat it as a distinct SST file. // ... as long as we did actually treat it as a distinct SST file.
EXPECT_EQ(children.size(), 6U); // Another SST and blob added EXPECT_EQ(children.size(), 3U); // Another SST added
} }
CloseDBAndBackupEngine(); CloseDBAndBackupEngine();
ASSERT_OK(DestroyDB(dbname_, options_)); ASSERT_OK(DestroyDB(dbname_, options_));

Loading…
Cancel
Save