From bafec6bb30c8da09558f1af4278dcbe1defbdfa1 Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Wed, 10 Jan 2018 12:14:18 -0800 Subject: [PATCH] Fix checkpoint_test directory setup/cleanup Summary: - Change directory name from "db_test" to "checkpoint_test". Previously it used the same directory as `db_test` - Systematically cleanup snapshot and snapshot staging directories before each test. Previously a failed test run caused subsequent runs to fail, particularly when the first failure caused "snapshot.tmp" to not be cleaned up. Closes https://github.com/facebook/rocksdb/pull/3351 Differential Revision: D6691015 Pulled By: ajkr fbshipit-source-id: 4fc2ac2e21ff2617ea0e96297c5132b5f2eefd79 --- utilities/checkpoint/checkpoint_test.cc | 54 ++++++++++--------------- 1 file changed, 21 insertions(+), 33 deletions(-) diff --git a/utilities/checkpoint/checkpoint_test.cc b/utilities/checkpoint/checkpoint_test.cc index 56c8c6e05..ad49fd55c 100644 --- a/utilities/checkpoint/checkpoint_test.cc +++ b/utilities/checkpoint/checkpoint_test.cc @@ -42,11 +42,12 @@ class CheckpointTest : public testing::Test { DB* db_; Options last_options_; std::vector handles_; + std::string snapshot_name_; CheckpointTest() : env_(Env::Default()) { env_->SetBackgroundThreads(1, Env::LOW); env_->SetBackgroundThreads(1, Env::HIGH); - dbname_ = test::TmpDir(env_) + "/db_test"; + dbname_ = test::TmpDir(env_) + "/checkpoint_test"; alternative_wal_dir_ = dbname_ + "/wal"; auto options = CurrentOptions(); auto delete_options = options; @@ -55,6 +56,12 @@ class CheckpointTest : public testing::Test { // Destroy it for not alternative WAL dir is used. EXPECT_OK(DestroyDB(dbname_, options)); db_ = nullptr; + snapshot_name_ = test::TmpDir(env_) + "/snapshot"; + std::string snapshot_tmp_name = snapshot_name_ + ".tmp"; + EXPECT_OK(DestroyDB(snapshot_name_, options)); + env_->DeleteDir(snapshot_name_); + EXPECT_OK(DestroyDB(snapshot_tmp_name, options)); + env_->DeleteDir(snapshot_tmp_name); Reopen(options); } @@ -69,6 +76,7 @@ class CheckpointTest : public testing::Test { options.db_paths.emplace_back(dbname_ + "_3", 0); options.db_paths.emplace_back(dbname_ + "_4", 0); EXPECT_OK(DestroyDB(dbname_, options)); + EXPECT_OK(DestroyDB(snapshot_name_, options)); } // Return the current option configuration. @@ -219,7 +227,6 @@ class CheckpointTest : public testing::Test { TEST_F(CheckpointTest, GetSnapshotLink) { for (uint64_t log_size_for_flush : {0, 1000000}) { Options options; - const std::string snapshot_name = test::TmpDir(env_) + "/snapshot"; DB* snapshotDB; ReadOptions roptions; std::string result; @@ -229,8 +236,6 @@ TEST_F(CheckpointTest, GetSnapshotLink) { delete db_; db_ = nullptr; ASSERT_OK(DestroyDB(dbname_, options)); - ASSERT_OK(DestroyDB(snapshot_name, options)); - env_->DeleteDir(snapshot_name); // Create a database Status s; @@ -240,14 +245,14 @@ TEST_F(CheckpointTest, GetSnapshotLink) { ASSERT_OK(Put(key, "v1")); // Take a snapshot ASSERT_OK(Checkpoint::Create(db_, &checkpoint)); - ASSERT_OK(checkpoint->CreateCheckpoint(snapshot_name, log_size_for_flush)); + ASSERT_OK(checkpoint->CreateCheckpoint(snapshot_name_, log_size_for_flush)); ASSERT_OK(Put(key, "v2")); ASSERT_EQ("v2", Get(key)); ASSERT_OK(Flush()); ASSERT_EQ("v2", Get(key)); // Open snapshot and verify contents while DB is running options.create_if_missing = false; - ASSERT_OK(DB::Open(options, snapshot_name, &snapshotDB)); + ASSERT_OK(DB::Open(options, snapshot_name_, &snapshotDB)); ASSERT_OK(snapshotDB->Get(roptions, key, &result)); ASSERT_EQ("v1", result); delete snapshotDB; @@ -260,7 +265,7 @@ TEST_F(CheckpointTest, GetSnapshotLink) { // Open snapshot and verify contents options.create_if_missing = false; - dbname_ = snapshot_name; + dbname_ = snapshot_name_; ASSERT_OK(DB::Open(options, dbname_, &db_)); ASSERT_EQ("v1", Get(key)); delete db_; @@ -289,21 +294,17 @@ TEST_F(CheckpointTest, CheckpointCF) { ASSERT_OK(Put(4, "four", "four")); ASSERT_OK(Put(5, "five", "five")); - const std::string snapshot_name = test::TmpDir(env_) + "/snapshot"; DB* snapshotDB; ReadOptions roptions; std::string result; std::vector cphandles; - ASSERT_OK(DestroyDB(snapshot_name, options)); - env_->DeleteDir(snapshot_name); - Status s; // Take a snapshot rocksdb::port::Thread t([&]() { Checkpoint* checkpoint; ASSERT_OK(Checkpoint::Create(db_, &checkpoint)); - ASSERT_OK(checkpoint->CreateCheckpoint(snapshot_name)); + ASSERT_OK(checkpoint->CreateCheckpoint(snapshot_name_)); delete checkpoint; }); TEST_SYNC_POINT("CheckpointTest::CheckpointCF:1"); @@ -331,7 +332,7 @@ TEST_F(CheckpointTest, CheckpointCF) { for (size_t i = 0; i < cfs.size(); ++i) { column_families.push_back(ColumnFamilyDescriptor(cfs[i], options)); } - ASSERT_OK(DB::Open(options, snapshot_name, + ASSERT_OK(DB::Open(options, snapshot_name_, column_families, &cphandles, &snapshotDB)); ASSERT_OK(snapshotDB->Get(roptions, cphandles[0], "Default", &result)); ASSERT_EQ("Default1", result); @@ -344,7 +345,6 @@ TEST_F(CheckpointTest, CheckpointCF) { cphandles.clear(); delete snapshotDB; snapshotDB = nullptr; - ASSERT_OK(DestroyDB(snapshot_name, options)); } TEST_F(CheckpointTest, CheckpointCFNoFlush) { @@ -358,15 +358,11 @@ TEST_F(CheckpointTest, CheckpointCFNoFlush) { Flush(); ASSERT_OK(Put(2, "two", "two")); - const std::string snapshot_name = test::TmpDir(env_) + "/snapshot"; DB* snapshotDB; ReadOptions roptions; std::string result; std::vector cphandles; - ASSERT_OK(DestroyDB(snapshot_name, options)); - env_->DeleteDir(snapshot_name); - Status s; // Take a snapshot rocksdb::SyncPoint::GetInstance()->SetCallBack( @@ -377,7 +373,7 @@ TEST_F(CheckpointTest, CheckpointCFNoFlush) { rocksdb::SyncPoint::GetInstance()->EnableProcessing(); Checkpoint* checkpoint; ASSERT_OK(Checkpoint::Create(db_, &checkpoint)); - ASSERT_OK(checkpoint->CreateCheckpoint(snapshot_name, 1000000)); + ASSERT_OK(checkpoint->CreateCheckpoint(snapshot_name_, 1000000)); rocksdb::SyncPoint::GetInstance()->DisableProcessing(); delete checkpoint; @@ -395,7 +391,7 @@ TEST_F(CheckpointTest, CheckpointCFNoFlush) { for (size_t i = 0; i < cfs.size(); ++i) { column_families.push_back(ColumnFamilyDescriptor(cfs[i], options)); } - ASSERT_OK(DB::Open(options, snapshot_name, column_families, &cphandles, + ASSERT_OK(DB::Open(options, snapshot_name_, column_families, &cphandles, &snapshotDB)); ASSERT_OK(snapshotDB->Get(roptions, cphandles[0], "Default", &result)); ASSERT_EQ("Default", result); @@ -409,14 +405,9 @@ TEST_F(CheckpointTest, CheckpointCFNoFlush) { cphandles.clear(); delete snapshotDB; snapshotDB = nullptr; - ASSERT_OK(DestroyDB(snapshot_name, options)); } TEST_F(CheckpointTest, CurrentFileModifiedWhileCheckpointing) { - const std::string kSnapshotName = test::TmpDir(env_) + "/snapshot"; - ASSERT_OK(DestroyDB(kSnapshotName, CurrentOptions())); - env_->DeleteDir(kSnapshotName); - Options options = CurrentOptions(); options.max_manifest_file_size = 0; // always rollover manifest for file add Reopen(options); @@ -438,7 +429,7 @@ TEST_F(CheckpointTest, CurrentFileModifiedWhileCheckpointing) { rocksdb::port::Thread t([&]() { Checkpoint* checkpoint; ASSERT_OK(Checkpoint::Create(db_, &checkpoint)); - ASSERT_OK(checkpoint->CreateCheckpoint(kSnapshotName)); + ASSERT_OK(checkpoint->CreateCheckpoint(snapshot_name_)); delete checkpoint; }); TEST_SYNC_POINT( @@ -452,18 +443,15 @@ TEST_F(CheckpointTest, CurrentFileModifiedWhileCheckpointing) { DB* snapshotDB; // Successful Open() implies that CURRENT pointed to the manifest in the // checkpoint. - ASSERT_OK(DB::Open(options, kSnapshotName, &snapshotDB)); + ASSERT_OK(DB::Open(options, snapshot_name_, &snapshotDB)); delete snapshotDB; snapshotDB = nullptr; } TEST_F(CheckpointTest, CurrentFileModifiedWhileCheckpointing2PC) { Close(); - const std::string kSnapshotName = test::TmpDir(env_) + "/snapshot"; const std::string dbname = test::TmpDir() + "/transaction_testdb"; - ASSERT_OK(DestroyDB(kSnapshotName, CurrentOptions())); ASSERT_OK(DestroyDB(dbname, CurrentOptions())); - env_->DeleteDir(kSnapshotName); env_->DeleteDir(dbname); Options options = CurrentOptions(); @@ -521,7 +509,7 @@ TEST_F(CheckpointTest, CurrentFileModifiedWhileCheckpointing2PC) { rocksdb::port::Thread t([&]() { Checkpoint* checkpoint; ASSERT_OK(Checkpoint::Create(txdb, &checkpoint)); - ASSERT_OK(checkpoint->CreateCheckpoint(kSnapshotName)); + ASSERT_OK(checkpoint->CreateCheckpoint(snapshot_name_)); delete checkpoint; }); TEST_SYNC_POINT( @@ -536,7 +524,7 @@ TEST_F(CheckpointTest, CurrentFileModifiedWhileCheckpointing2PC) { // No more than two logs files should exist. std::vector files; - env_->GetChildren(kSnapshotName, &files); + env_->GetChildren(snapshot_name_, &files); int num_log_files = 0; for (auto& file : files) { uint64_t num; @@ -559,7 +547,7 @@ TEST_F(CheckpointTest, CurrentFileModifiedWhileCheckpointing2PC) { column_families.push_back( ColumnFamilyDescriptor("CFB", ColumnFamilyOptions())); std::vector cf_handles; - ASSERT_OK(TransactionDB::Open(options, txn_db_options, kSnapshotName, + ASSERT_OK(TransactionDB::Open(options, txn_db_options, snapshot_name_, column_families, &cf_handles, &snapshotDB)); ASSERT_OK(snapshotDB->Get(read_options, "foo", &value)); ASSERT_EQ(value, "bar");