From 58d46d19158c95a4f05e3609579e65e9181b19cb Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Fri, 13 Dec 2019 13:25:14 -0800 Subject: [PATCH] Add useful idioms to Random API (OneInOpt, PercentTrue) (#6154) Summary: And clean up related code, especially in stress test. (More clean up of db_stress_test_base.cc coming after this.) Pull Request resolved: https://github.com/facebook/rocksdb/pull/6154 Test Plan: make check, make blackbox_crash_test for a bit Differential Revision: D18938180 Pulled By: pdillinger fbshipit-source-id: 524d27621b8dbb25f6dff40f1081e7c00630357e --- CMakeLists.txt | 1 + Makefile | 4 + TARGETS | 7 ++ db/db_iterator_test.cc | 10 +-- db_stress_tool/db_stress_test_base.cc | 49 +++++------ db_stress_tool/no_batched_ops_stress.cc | 4 +- src.mk | 1 + util/random.h | 13 ++- util/random_test.cc | 105 ++++++++++++++++++++++++ 9 files changed, 159 insertions(+), 35 deletions(-) create mode 100644 util/random_test.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index 9bb3b4541..881384264 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1028,6 +1028,7 @@ if(WITH_TESTS) util/filelock_test.cc util/hash_test.cc util/heap_test.cc + util/random_test.cc util/rate_limiter_test.cc util/repeatable_thread_test.cc util/slice_transform_test.cc diff --git a/Makefile b/Makefile index 3bed684eb..e4cbedd0b 100644 --- a/Makefile +++ b/Makefile @@ -460,6 +460,7 @@ TESTS = \ env_test \ env_logger_test \ hash_test \ + random_test \ thread_local_test \ rate_limiter_test \ perf_context_test \ @@ -1245,6 +1246,9 @@ coding_test: util/coding_test.o $(LIBOBJECTS) $(TESTHARNESS) hash_test: util/hash_test.o $(LIBOBJECTS) $(TESTHARNESS) $(AM_LINK) +random_test: util/random_test.o $(LIBOBJECTS) $(TESTHARNESS) + $(AM_LINK) + option_change_migration_test: utilities/option_change_migration/option_change_migration_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTHARNESS) $(AM_LINK) diff --git a/TARGETS b/TARGETS index 9bd7c819c..2cd920449 100644 --- a/TARGETS +++ b/TARGETS @@ -1245,6 +1245,13 @@ ROCKS_TESTS = [ [], [], ], + [ + "random_test", + "util/random_test.cc", + "serial", + [], + [], + ], [ "range_del_aggregator_test", "db/range_del_aggregator_test.cc", diff --git a/db/db_iterator_test.cc b/db/db_iterator_test.cc index 6abe40b27..3fadd55dd 100644 --- a/db/db_iterator_test.cc +++ b/db/db_iterator_test.cc @@ -1297,19 +1297,19 @@ class DBIteratorTestForPinnedData : public DBIteratorTest { // Insert data to true_data map and to DB true_data[k] = v; - if (rnd.OneIn(static_cast(100.0 / merge_percentage))) { + if (rnd.PercentTrue(merge_percentage)) { ASSERT_OK(db_->Merge(WriteOptions(), k, v)); } else { ASSERT_OK(Put(k, v)); } // Pick random keys to be used to test Seek() - if (rnd.OneIn(static_cast(100.0 / seeks_percentage))) { + if (rnd.PercentTrue(seeks_percentage)) { random_keys.push_back(k); } // Delete some random keys - if (rnd.OneIn(static_cast(100.0 / delete_percentage))) { + if (rnd.PercentTrue(delete_percentage)) { deleted_keys.push_back(k); true_data.erase(k); ASSERT_OK(Delete(k)); @@ -1867,7 +1867,7 @@ TEST_P(DBIteratorTest, IterPrevKeyCrossingBlocksRandomized) { // make sure that we dont merge them while flushing but actually // merge them in the read path for (int i = 0; i < kNumKeys; i++) { - if (rnd.OneIn(static_cast(100.0 / kNoMergeOpPercentage))) { + if (rnd.PercentTrue(kNoMergeOpPercentage)) { // Dont give merge operations for some keys continue; } @@ -1883,7 +1883,7 @@ TEST_P(DBIteratorTest, IterPrevKeyCrossingBlocksRandomized) { ASSERT_OK(Flush()); for (int i = 0; i < kNumKeys; i++) { - if (rnd.OneIn(static_cast(100.0 / kDeletePercentage))) { + if (rnd.PercentTrue(kDeletePercentage)) { gen_key = Key(i); ASSERT_OK(Delete(gen_key)); diff --git a/db_stress_tool/db_stress_test_base.cc b/db_stress_tool/db_stress_test_base.cc index 390ea6750..112258ff7 100644 --- a/db_stress_tool/db_stress_test_base.cc +++ b/db_stress_tool/db_stress_test_base.cc @@ -501,20 +501,17 @@ void StressTest::OperateDb(ThreadState* thread) { } // Change Options - if (FLAGS_set_options_one_in > 0 && - thread->rand.OneIn(FLAGS_set_options_one_in)) { + if (thread->rand.OneInOpt(FLAGS_set_options_one_in)) { SetOptions(thread); } - if (FLAGS_set_in_place_one_in > 0 && - thread->rand.OneIn(FLAGS_set_in_place_one_in)) { + if (thread->rand.OneInOpt(FLAGS_set_in_place_one_in)) { options_.inplace_update_support ^= options_.inplace_update_support; } MaybeClearOneColumnFamily(thread); - if (FLAGS_sync_wal_one_in > 0 && - thread->rand.Uniform(FLAGS_sync_wal_one_in) == 0) { + if (thread->rand.OneInOpt(FLAGS_sync_wal_one_in)) { Status s = db_->SyncWAL(); if (!s.ok() && !s.IsNotSupported()) { fprintf(stdout, "SyncWAL() failed: %s\n", s.ToString().c_str()); @@ -522,8 +519,7 @@ void StressTest::OperateDb(ThreadState* thread) { } #ifndef ROCKSDB_LITE - if (FLAGS_compact_files_one_in > 0 && - thread->rand.Uniform(FLAGS_compact_files_one_in) == 0) { + if (thread->rand.OneInOpt(FLAGS_compact_files_one_in)) { auto* random_cf = column_families_[thread->rand.Next() % FLAGS_column_families]; rocksdb::ColumnFamilyMetaData cf_meta_data; @@ -584,8 +580,7 @@ void StressTest::OperateDb(ThreadState* thread) { ColumnFamilyHandle* column_family = column_families_[rand_column_family]; - if (FLAGS_compact_range_one_in > 0 && - thread->rand.Uniform(FLAGS_compact_range_one_in) == 0) { + if (thread->rand.OneInOpt(FLAGS_compact_range_one_in)) { TestCompactRange(thread, rand_key, key, column_family); if (thread->shared->HasVerificationFailedYet()) { break; @@ -595,8 +590,7 @@ void StressTest::OperateDb(ThreadState* thread) { std::vector rand_column_families = GenerateColumnFamilies(FLAGS_column_families, rand_column_family); - if (FLAGS_flush_one_in > 0 && - thread->rand.Uniform(FLAGS_flush_one_in) == 0) { + if (thread->rand.OneInOpt(FLAGS_flush_one_in)) { FlushOptions flush_opts; std::vector cfhs; std::for_each( @@ -609,8 +603,7 @@ void StressTest::OperateDb(ThreadState* thread) { } } - if (FLAGS_pause_background_one_in > 0 && - thread->rand.OneIn(FLAGS_pause_background_one_in)) { + if (thread->rand.OneInOpt(FLAGS_pause_background_one_in)) { Status status = db_->PauseBackgroundWork(); if (!status.ok()) { VerificationAbort(shared, "PauseBackgroundWork status not OK", @@ -633,13 +626,11 @@ void StressTest::OperateDb(ThreadState* thread) { std::vector rand_keys = GenerateKeys(rand_key); - if (FLAGS_ingest_external_file_one_in > 0 && - thread->rand.Uniform(FLAGS_ingest_external_file_one_in) == 0) { + if (thread->rand.OneInOpt(FLAGS_ingest_external_file_one_in)) { TestIngestExternalFile(thread, rand_column_families, rand_keys, lock); } - if (FLAGS_backup_one_in > 0 && - thread->rand.Uniform(FLAGS_backup_one_in) == 0) { + if (thread->rand.OneInOpt(FLAGS_backup_one_in)) { Status s = TestBackupRestore(thread, rand_column_families, rand_keys); if (!s.ok()) { VerificationAbort(shared, "Backup/restore gave inconsistent state", @@ -647,16 +638,14 @@ void StressTest::OperateDb(ThreadState* thread) { } } - if (FLAGS_checkpoint_one_in > 0 && - thread->rand.Uniform(FLAGS_checkpoint_one_in) == 0) { + if (thread->rand.OneInOpt(FLAGS_checkpoint_one_in)) { Status s = TestCheckpoint(thread, rand_column_families, rand_keys); if (!s.ok()) { VerificationAbort(shared, "Checkpoint gave inconsistent state", s); } } - if (FLAGS_acquire_snapshot_one_in > 0 && - thread->rand.Uniform(FLAGS_acquire_snapshot_one_in) == 0) { + if (thread->rand.OneInOpt(FLAGS_acquire_snapshot_one_in)) { #ifndef ROCKSDB_LITE auto db_impl = reinterpret_cast(db_->GetRootDB()); const bool ww_snapshot = thread->rand.OneIn(10); @@ -718,7 +707,8 @@ void StressTest::OperateDb(ThreadState* thread) { // Reset this in case we pick something other than a read op. We don't // want to use a stale value when deciding at the beginning of the loop // whether to vote to reopen - if (prob_op >= 0 && prob_op < (int)FLAGS_readpercent) { + if (prob_op < (int)FLAGS_readpercent) { + assert(0 <= prob_op); // OPERATION read if (FLAGS_use_multiget) { // Leave room for one more iteration of the loop with a single key @@ -735,25 +725,30 @@ void StressTest::OperateDb(ThreadState* thread) { } else { TestGet(thread, read_opts, rand_column_families, rand_keys); } - } else if ((int)FLAGS_readpercent <= prob_op && prob_op < prefixBound) { + } else if (prob_op < prefixBound) { + assert((int)FLAGS_readpercent <= prob_op); // OPERATION prefix scan // keys are 8 bytes long, prefix size is FLAGS_prefix_size. There are // (8 - FLAGS_prefix_size) bytes besides the prefix. So there will // be 2 ^ ((8 - FLAGS_prefix_size) * 8) possible keys with the same // prefix TestPrefixScan(thread, read_opts, rand_column_families, rand_keys); - } else if (prefixBound <= prob_op && prob_op < writeBound) { + } else if (prob_op < writeBound) { + assert(prefixBound <= prob_op); // OPERATION write TestPut(thread, write_opts, read_opts, rand_column_families, rand_keys, value, lock); - } else if (writeBound <= prob_op && prob_op < delBound) { + } else if (prob_op < delBound) { + assert(writeBound <= prob_op); // OPERATION delete TestDelete(thread, write_opts, rand_column_families, rand_keys, lock); - } else if (delBound <= prob_op && prob_op < delRangeBound) { + } else if (prob_op < delRangeBound) { + assert(delBound <= prob_op); // OPERATION delete range TestDeleteRange(thread, write_opts, rand_column_families, rand_keys, lock); } else { + assert(delRangeBound <= prob_op); // OPERATION iterate int num_seeks = static_cast( std::min(static_cast(thread->rand.Uniform(4)), diff --git a/db_stress_tool/no_batched_ops_stress.cc b/db_stress_tool/no_batched_ops_stress.cc index 17f6f6efd..f75339737 100644 --- a/db_stress_tool/no_batched_ops_stress.cc +++ b/db_stress_tool/no_batched_ops_stress.cc @@ -96,8 +96,8 @@ class NonBatchedOpsStressTest : public StressTest { } 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)) { + if (FLAGS_column_families > 1) { + if (thread->rand.OneInOpt(FLAGS_clear_column_family_one_in)) { // drop column family and then create it again (can't drop default) int cf = thread->rand.Next() % (FLAGS_column_families - 1) + 1; std::string new_name = ToString(new_column_family_name_.fetch_add(1)); diff --git a/src.mk b/src.mk index ec5c722eb..9498d45e3 100644 --- a/src.mk +++ b/src.mk @@ -420,6 +420,7 @@ MAIN_SOURCES = \ util/filelock_test.cc \ util/log_write_bench.cc \ util/rate_limiter_test.cc \ + util/random_test.cc \ util/repeatable_thread_test.cc \ util/slice_transform_test.cc \ util/timer_queue_test.cc \ diff --git a/util/random.h b/util/random.h index f5beb9cd2..d1031bff7 100644 --- a/util/random.h +++ b/util/random.h @@ -63,7 +63,18 @@ class Random { // Randomly returns true ~"1/n" of the time, and false otherwise. // REQUIRES: n > 0 - bool OneIn(int n) { return (Next() % n) == 0; } + bool OneIn(int n) { return Uniform(n) == 0; } + + // "Optional" one-in-n, where 0 or negative always returns false + // (may or may not consume a random value) + bool OneInOpt(int n) { return n > 0 && OneIn(n); } + + // Returns random bool that is true for the given percentage of + // calls on average. Zero or less is always false and 100 or more + // is always true (may or may not consume a random value) + bool PercentTrue(int percentage) { + return static_cast(Uniform(100)) < percentage; + } // Skewed: pick "base" uniformly from range [0,max_log] and then // return "base" random bits. The effect is to pick a number in the diff --git a/util/random_test.cc b/util/random_test.cc new file mode 100644 index 000000000..244b73444 --- /dev/null +++ b/util/random_test.cc @@ -0,0 +1,105 @@ +// Copyright (c) 2011-present, Facebook, Inc. All rights reserved. +// This source code is licensed under both the GPLv2 (found in the +// COPYING file in the root directory) and Apache 2.0 License +// (found in the LICENSE.Apache file in the root directory). +// +// Copyright (c) 2012 The LevelDB Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. See the AUTHORS file for names of contributors. + +#include +#include + +#include "test_util/testharness.h" +#include "util/random.h" + +using rocksdb::Random; + +TEST(RandomTest, Uniform) { + const int average = 20; + for (uint32_t seed : {0, 1, 2, 37, 4096}) { + Random r(seed); + for (int range : {1, 2, 8, 12, 100}) { + std::vector counts(range, 0); + + for (int i = 0; i < range * average; ++i) { + ++counts.at(r.Uniform(range)); + } + int max_variance = static_cast(std::sqrt(range) * 2 + 4); + for (int i = 0; i < range; ++i) { + EXPECT_GE(counts[i], std::max(1, average - max_variance)); + EXPECT_LE(counts[i], average + max_variance + 1); + } + } + } +} + +TEST(RandomTest, OneIn) { + Random r(42); + for (int range : {1, 2, 8, 12, 100, 1234}) { + const int average = 100; + int count = 0; + for (int i = 0; i < average * range; ++i) { + if (r.OneIn(range)) { + ++count; + } + } + if (range == 1) { + EXPECT_EQ(count, average); + } else { + int max_variance = static_cast(std::sqrt(average) * 1.5); + EXPECT_GE(count, average - max_variance); + EXPECT_LE(count, average + max_variance); + } + } +} + +TEST(RandomTest, OneInOpt) { + Random r(42); + for (int range : {-12, 0, 1, 2, 8, 12, 100, 1234}) { + const int average = 100; + int count = 0; + for (int i = 0; i < average * range; ++i) { + if (r.OneInOpt(range)) { + ++count; + } + } + if (range < 1) { + EXPECT_EQ(count, 0); + } else if (range == 1) { + EXPECT_EQ(count, average); + } else { + int max_variance = static_cast(std::sqrt(average) * 1.5); + EXPECT_GE(count, average - max_variance); + EXPECT_LE(count, average + max_variance); + } + } +} + +TEST(RandomTest, PercentTrue) { + Random r(42); + for (int pct : {-12, 0, 1, 2, 10, 50, 90, 98, 99, 100, 1234}) { + const int samples = 10000; + + int count = 0; + for (int i = 0; i < samples; ++i) { + if (r.PercentTrue(pct)) { + ++count; + } + } + if (pct <= 0) { + EXPECT_EQ(count, 0); + } else if (pct >= 100) { + EXPECT_EQ(count, samples); + } else { + int est = (count * 100 + (samples / 2)) / samples; + EXPECT_EQ(est, pct); + } + } +} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + + return RUN_ALL_TESTS(); +}