From a960287dee4fef9157a94fb6508f6ddbac940b85 Mon Sep 17 00:00:00 2001 From: sdong Date: Mon, 9 Dec 2019 14:36:10 -0800 Subject: [PATCH] db_stress: Some code style improvements (#6137) Summary: Two changes: 1. Prevent static variables in a header file 2. Add "override" keyword when virtual functions are overridden. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6137 Test Plan: Build db_stress with or without LITE. Differential Revision: D18892007 fbshipit-source-id: 295356427a34473b23ed36d6ed4ef3ae35a32db0 --- db_stress_tool/batched_ops_stress.cc | 52 +++++++++-------- db_stress_tool/cf_consistency_stress.cc | 74 ++++++++++++------------- db_stress_tool/db_stress_common.h | 7 +-- db_stress_tool/db_stress_tool.cc | 4 ++ db_stress_tool/no_batched_ops_stress.cc | 60 ++++++++++---------- 5 files changed, 98 insertions(+), 99 deletions(-) diff --git a/db_stress_tool/batched_ops_stress.cc b/db_stress_tool/batched_ops_stress.cc index 3f789616ff..e5e346da9d 100644 --- a/db_stress_tool/batched_ops_stress.cc +++ b/db_stress_tool/batched_ops_stress.cc @@ -19,12 +19,11 @@ class BatchedOpsStressTest : public StressTest { // Given a key K and value V, this puts ("0"+K, "0"+V), ("1"+K, "1"+V), ... // ("9"+K, "9"+V) in DB atomically i.e in a single batch. // Also refer BatchedOpsStressTest::TestGet - virtual Status TestPut(ThreadState* thread, WriteOptions& write_opts, - const ReadOptions& /* read_opts */, - const std::vector& rand_column_families, - const std::vector& rand_keys, - char (&value)[100], - std::unique_ptr& /* lock */) { + Status TestPut(ThreadState* thread, WriteOptions& write_opts, + const ReadOptions& /* read_opts */, + const std::vector& rand_column_families, + const std::vector& rand_keys, char (&value)[100], + std::unique_ptr& /* lock */) override { uint32_t value_base = thread->rand.Next() % thread->shared->UNKNOWN_SENTINEL; size_t sz = GenerateValue(value_base, value, sizeof(value)); @@ -61,10 +60,10 @@ class BatchedOpsStressTest : public StressTest { // Given a key K, this deletes ("0"+K), ("1"+K),... ("9"+K) // in DB atomically i.e in a single batch. Also refer MultiGet. - virtual Status TestDelete(ThreadState* thread, WriteOptions& writeoptions, - const std::vector& rand_column_families, - const std::vector& rand_keys, - std::unique_ptr& /* lock */) { + Status TestDelete(ThreadState* thread, WriteOptions& writeoptions, + const std::vector& rand_column_families, + const std::vector& rand_keys, + std::unique_ptr& /* lock */) override { std::string keys[10] = {"9", "7", "5", "3", "1", "8", "6", "4", "2", "0"}; WriteBatch batch; @@ -87,22 +86,22 @@ class BatchedOpsStressTest : public StressTest { return s; } - virtual Status TestDeleteRange( - ThreadState* /* thread */, WriteOptions& /* write_opts */, - const std::vector& /* rand_column_families */, - const std::vector& /* rand_keys */, - std::unique_ptr& /* lock */) { + Status TestDeleteRange(ThreadState* /* thread */, + WriteOptions& /* write_opts */, + const std::vector& /* rand_column_families */, + const std::vector& /* rand_keys */, + std::unique_ptr& /* lock */) override { assert(false); return Status::NotSupported( "BatchedOpsStressTest does not support " "TestDeleteRange"); } - virtual void TestIngestExternalFile( + void TestIngestExternalFile( ThreadState* /* thread */, const std::vector& /* rand_column_families */, const std::vector& /* rand_keys */, - std::unique_ptr& /* lock */) { + std::unique_ptr& /* lock */) override { assert(false); fprintf(stderr, "BatchedOpsStressTest does not support " @@ -115,9 +114,9 @@ class BatchedOpsStressTest : public StressTest { // "0"+V, "1"+V,..."9"+V. // ASSUMES that BatchedOpsStressTest::TestPut was used to put (K, V) into // the DB. - virtual Status TestGet(ThreadState* thread, const ReadOptions& readoptions, - const std::vector& rand_column_families, - const std::vector& rand_keys) { + Status TestGet(ThreadState* thread, const ReadOptions& readoptions, + const std::vector& rand_column_families, + const std::vector& rand_keys) override { std::string keys[10] = {"0", "1", "2", "3", "4", "5", "6", "7", "8", "9"}; Slice key_slices[10]; std::string values[10]; @@ -170,10 +169,10 @@ class BatchedOpsStressTest : public StressTest { return s; } - virtual std::vector TestMultiGet( + std::vector TestMultiGet( ThreadState* thread, const ReadOptions& readoptions, const std::vector& rand_column_families, - const std::vector& rand_keys) { + const std::vector& rand_keys) override { size_t num_keys = rand_keys.size(); std::vector ret_status(num_keys); std::array keys = {"0", "1", "2", "3", "4", @@ -246,10 +245,9 @@ class BatchedOpsStressTest : public StressTest { // each series should be the same length, and it is verified for each // index i that all the i'th values are of the form "0"+V, "1"+V,..."9"+V. // ASSUMES that MultiPut was used to put (K, V) - virtual Status TestPrefixScan(ThreadState* thread, - const ReadOptions& readoptions, - const std::vector& rand_column_families, - const std::vector& rand_keys) { + Status TestPrefixScan(ThreadState* thread, const ReadOptions& readoptions, + const std::vector& rand_column_families, + const std::vector& rand_keys) override { size_t prefix_to_use = (FLAGS_prefix_size < 0) ? 7 : static_cast(FLAGS_prefix_size); std::string key_str = Key(rand_keys[0]); @@ -333,7 +331,7 @@ class BatchedOpsStressTest : public StressTest { return s; } - virtual void VerifyDb(ThreadState* /* thread */) const {} + void VerifyDb(ThreadState* /* thread */) const override {} }; StressTest* CreateBatchedOpsStressTest() { return new BatchedOpsStressTest(); } diff --git a/db_stress_tool/cf_consistency_stress.cc b/db_stress_tool/cf_consistency_stress.cc index e84a3702cf..34349982f4 100644 --- a/db_stress_tool/cf_consistency_stress.cc +++ b/db_stress_tool/cf_consistency_stress.cc @@ -17,12 +17,11 @@ class CfConsistencyStressTest : public StressTest { virtual ~CfConsistencyStressTest() {} - virtual Status TestPut(ThreadState* thread, WriteOptions& write_opts, - const ReadOptions& /* read_opts */, - const std::vector& rand_column_families, - const std::vector& rand_keys, - char (&value)[100], - std::unique_ptr& /* lock */) { + Status TestPut(ThreadState* thread, WriteOptions& write_opts, + const ReadOptions& /* read_opts */, + const std::vector& rand_column_families, + const std::vector& rand_keys, char (&value)[100], + std::unique_ptr& /* lock */) override { std::string key_str = Key(rand_keys[0]); Slice key = key_str; uint64_t value_base = batch_id_.fetch_add(1); @@ -50,10 +49,10 @@ class CfConsistencyStressTest : public StressTest { return s; } - virtual Status TestDelete(ThreadState* thread, WriteOptions& write_opts, - const std::vector& rand_column_families, - const std::vector& rand_keys, - std::unique_ptr& /* lock */) { + Status TestDelete(ThreadState* thread, WriteOptions& write_opts, + const std::vector& rand_column_families, + const std::vector& rand_keys, + std::unique_ptr& /* lock */) override { std::string key_str = Key(rand_keys[0]); Slice key = key_str; WriteBatch batch; @@ -71,10 +70,10 @@ class CfConsistencyStressTest : public StressTest { return s; } - virtual Status TestDeleteRange(ThreadState* thread, WriteOptions& write_opts, - const std::vector& rand_column_families, - const std::vector& rand_keys, - std::unique_ptr& /* lock */) { + Status TestDeleteRange(ThreadState* thread, WriteOptions& write_opts, + const std::vector& rand_column_families, + const std::vector& rand_keys, + std::unique_ptr& /* lock */) override { int64_t rand_key = rand_keys[0]; auto shared = thread->shared; int64_t max_key = shared->GetMaxKey(); @@ -102,11 +101,11 @@ class CfConsistencyStressTest : public StressTest { return s; } - virtual void TestIngestExternalFile( + void TestIngestExternalFile( ThreadState* /* thread */, const std::vector& /* rand_column_families */, const std::vector& /* rand_keys */, - std::unique_ptr& /* lock */) { + std::unique_ptr& /* lock */) override { assert(false); fprintf(stderr, "CfConsistencyStressTest does not support TestIngestExternalFile " @@ -114,9 +113,9 @@ class CfConsistencyStressTest : public StressTest { std::terminate(); } - virtual Status TestGet(ThreadState* thread, const ReadOptions& readoptions, - const std::vector& rand_column_families, - const std::vector& rand_keys) { + Status TestGet(ThreadState* thread, const ReadOptions& readoptions, + const std::vector& rand_column_families, + const std::vector& rand_keys) override { std::string key_str = Key(rand_keys[0]); Slice key = key_str; Status s; @@ -198,10 +197,10 @@ class CfConsistencyStressTest : public StressTest { return s; } - virtual std::vector TestMultiGet( + std::vector TestMultiGet( ThreadState* thread, const ReadOptions& read_opts, const std::vector& rand_column_families, - const std::vector& rand_keys) { + const std::vector& rand_keys) override { size_t num_keys = rand_keys.size(); std::vector key_str; std::vector keys; @@ -232,10 +231,9 @@ class CfConsistencyStressTest : public StressTest { return statuses; } - virtual Status TestPrefixScan(ThreadState* thread, - const ReadOptions& readoptions, - const std::vector& rand_column_families, - const std::vector& rand_keys) { + Status TestPrefixScan(ThreadState* thread, const ReadOptions& readoptions, + const std::vector& rand_column_families, + const std::vector& rand_keys) override { size_t prefix_to_use = (FLAGS_prefix_size < 0) ? 7 : static_cast(FLAGS_prefix_size); @@ -271,18 +269,17 @@ class CfConsistencyStressTest : public StressTest { return s; } - virtual ColumnFamilyHandle* GetControlCfh(ThreadState* thread, - int /*column_family_id*/ - ) { + ColumnFamilyHandle* GetControlCfh(ThreadState* thread, + int /*column_family_id*/ + ) override { // All column families should contain the same data. Randomly pick one. return column_families_[thread->rand.Next() % column_families_.size()]; } #ifdef ROCKSDB_LITE - virtual Status TestCheckpoint( - ThreadState* /* thread */, - const std::vector& /* rand_column_families */, - const std::vector& /* rand_keys */) { + Status TestCheckpoint(ThreadState* /* thread */, + const std::vector& /* rand_column_families */, + const std::vector& /* rand_keys */) override { assert(false); fprintf(stderr, "RocksDB lite does not support " @@ -290,9 +287,9 @@ class CfConsistencyStressTest : public StressTest { std::terminate(); } #else - virtual Status TestCheckpoint( - ThreadState* thread, const std::vector& /* rand_column_families */, - const std::vector& /* rand_keys */) { + Status TestCheckpoint(ThreadState* thread, + const std::vector& /* rand_column_families */, + const std::vector& /* rand_keys */) override { std::string checkpoint_dir = FLAGS_db + "/.checkpoint" + ToString(thread->tid); DestroyDB(checkpoint_dir, options_); @@ -338,7 +335,7 @@ class CfConsistencyStressTest : public StressTest { } #endif // !ROCKSDB_LITE - virtual void VerifyDb(ThreadState* thread) const { + void VerifyDb(ThreadState* thread) const override { ReadOptions options(FLAGS_verify_checksum, true); // We must set total_order_seek to true because we are doing a SeekToFirst // on a column family whose memtables may support (by default) prefix-based @@ -489,8 +486,9 @@ class CfConsistencyStressTest : public StressTest { } while (true); } - virtual std::vector GenerateColumnFamilies( - const int /* num_column_families */, int /* rand_column_family */) const { + std::vector GenerateColumnFamilies( + const int /* num_column_families */, + int /* rand_column_family */) const override { std::vector ret; int num = static_cast(column_families_.size()); int k = 0; diff --git a/db_stress_tool/db_stress_common.h b/db_stress_tool/db_stress_common.h index ae5cdf9f1b..2a1f340a2f 100644 --- a/db_stress_tool/db_stress_common.h +++ b/db_stress_tool/db_stress_common.h @@ -197,11 +197,10 @@ DECLARE_int32(prefix_size); DECLARE_bool(use_merge); DECLARE_bool(use_full_merge_v1); -static const long KB = 1024; -static const int kRandomValueMaxFactor = 3; -static const int kValueMaxLen = 100; +const long KB = 1024; +const int kRandomValueMaxFactor = 3; +const int kValueMaxLen = 100; -static std::shared_ptr env_guard; // posix or hdfs environment extern rocksdb::Env* FLAGS_env; diff --git a/db_stress_tool/db_stress_tool.cc b/db_stress_tool/db_stress_tool.cc index 46b4b170a9..4ce42d0dd6 100644 --- a/db_stress_tool/db_stress_tool.cc +++ b/db_stress_tool/db_stress_tool.cc @@ -25,6 +25,10 @@ #include "db_stress_tool/db_stress_driver.h" namespace rocksdb { +namespace { +static std::shared_ptr env_guard; +} // namespace + int db_stress_tool(int argc, char** argv) { SetUsageMessage(std::string("\nUSAGE:\n") + std::string(argv[0]) + " [OPTIONS]..."); diff --git a/db_stress_tool/no_batched_ops_stress.cc b/db_stress_tool/no_batched_ops_stress.cc index a8f9a86519..17f6f6efd2 100644 --- a/db_stress_tool/no_batched_ops_stress.cc +++ b/db_stress_tool/no_batched_ops_stress.cc @@ -17,7 +17,7 @@ class NonBatchedOpsStressTest : public StressTest { virtual ~NonBatchedOpsStressTest() {} - virtual void VerifyDb(ThreadState* thread) const { + void VerifyDb(ThreadState* thread) const override { ReadOptions options(FLAGS_verify_checksum, true); auto shared = thread->shared; const int64_t max_key = shared->GetMaxKey(); @@ -95,7 +95,7 @@ class NonBatchedOpsStressTest : public StressTest { } } - virtual void MaybeClearOneColumnFamily(ThreadState* thread) { + void MaybeClearOneColumnFamily(ThreadState* thread) override { if (FLAGS_clear_column_family_one_in != 0 && FLAGS_column_families > 1) { if (thread->rand.OneIn(FLAGS_clear_column_family_one_in)) { // drop column family and then create it again (can't drop default) @@ -130,11 +130,11 @@ class NonBatchedOpsStressTest : public StressTest { } } - virtual bool ShouldAcquireMutexOnKey() const { return true; } + bool ShouldAcquireMutexOnKey() const override { return true; } - virtual Status TestGet(ThreadState* thread, const ReadOptions& read_opts, - const std::vector& rand_column_families, - const std::vector& rand_keys) { + Status TestGet(ThreadState* thread, const ReadOptions& read_opts, + const std::vector& rand_column_families, + const std::vector& rand_keys) override { auto cfh = column_families_[rand_column_families[0]]; std::string key_str = Key(rand_keys[0]); Slice key = key_str; @@ -153,10 +153,10 @@ class NonBatchedOpsStressTest : public StressTest { return s; } - virtual std::vector TestMultiGet( + std::vector TestMultiGet( ThreadState* thread, const ReadOptions& read_opts, const std::vector& rand_column_families, - const std::vector& rand_keys) { + const std::vector& rand_keys) override { size_t num_keys = rand_keys.size(); std::vector key_str; std::vector keys; @@ -187,10 +187,9 @@ class NonBatchedOpsStressTest : public StressTest { return statuses; } - virtual Status TestPrefixScan(ThreadState* thread, - const ReadOptions& read_opts, - const std::vector& rand_column_families, - const std::vector& rand_keys) { + Status TestPrefixScan(ThreadState* thread, const ReadOptions& read_opts, + const std::vector& rand_column_families, + const std::vector& rand_keys) override { auto cfh = column_families_[rand_column_families[0]]; std::string key_str = Key(rand_keys[0]); Slice key = key_str; @@ -222,11 +221,11 @@ class NonBatchedOpsStressTest : public StressTest { return s; } - virtual Status TestPut(ThreadState* thread, WriteOptions& write_opts, - const ReadOptions& read_opts, - const std::vector& rand_column_families, - const std::vector& rand_keys, - char (&value)[100], std::unique_ptr& lock) { + Status TestPut(ThreadState* thread, WriteOptions& write_opts, + const ReadOptions& read_opts, + const std::vector& rand_column_families, + const std::vector& rand_keys, char (&value)[100], + std::unique_ptr& lock) override { auto shared = thread->shared; int64_t max_key = shared->GetMaxKey(); int64_t rand_key = rand_keys[0]; @@ -301,10 +300,10 @@ class NonBatchedOpsStressTest : public StressTest { return s; } - virtual Status TestDelete(ThreadState* thread, WriteOptions& write_opts, - const std::vector& rand_column_families, - const std::vector& rand_keys, - std::unique_ptr& lock) { + Status TestDelete(ThreadState* thread, WriteOptions& write_opts, + const std::vector& rand_column_families, + const std::vector& rand_keys, + std::unique_ptr& lock) override { int64_t rand_key = rand_keys[0]; int rand_column_family = rand_column_families[0]; auto shared = thread->shared; @@ -377,10 +376,10 @@ class NonBatchedOpsStressTest : public StressTest { return s; } - virtual Status TestDeleteRange(ThreadState* thread, WriteOptions& write_opts, - const std::vector& rand_column_families, - const std::vector& rand_keys, - std::unique_ptr& lock) { + Status TestDeleteRange(ThreadState* thread, WriteOptions& write_opts, + const std::vector& rand_column_families, + const std::vector& rand_keys, + std::unique_ptr& lock) override { // OPERATION delete range std::vector> range_locks; // delete range does not respect disallowed overwrites. the keys for @@ -429,11 +428,11 @@ class NonBatchedOpsStressTest : public StressTest { } #ifdef ROCKSDB_LITE - virtual void TestIngestExternalFile( + void TestIngestExternalFile( ThreadState* /* thread */, const std::vector& /* rand_column_families */, const std::vector& /* rand_keys */, - std::unique_ptr& /* lock */) { + std::unique_ptr& /* lock */) override { assert(false); fprintf(stderr, "RocksDB lite does not support " @@ -441,9 +440,10 @@ class NonBatchedOpsStressTest : public StressTest { std::terminate(); } #else - virtual void TestIngestExternalFile( - ThreadState* thread, const std::vector& rand_column_families, - const std::vector& rand_keys, std::unique_ptr& lock) { + void TestIngestExternalFile(ThreadState* thread, + const std::vector& rand_column_families, + const std::vector& rand_keys, + std::unique_ptr& lock) override { const std::string sst_filename = FLAGS_db + "/." + ToString(thread->tid) + ".sst"; Status s;