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

@@ -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

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)
 

TARGETS

@@ -1245,6 +1245,13 @@ ROCKS_TESTS = [
         [],
         [],
     ],
+    [
+        "random_test",
+        "util/random_test.cc",
+        "serial",
+        [],
+        [],
+    ],
     [
         "range_del_aggregator_test",
         "db/range_del_aggregator_test.cc",

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<int>(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<int>(100.0 / seeks_percentage))) {
+      if (rnd.PercentTrue(seeks_percentage)) {
         random_keys.push_back(k);
       }
 
       // Delete some random keys
-      if (rnd.OneIn(static_cast<int>(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<int>(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<int>(100.0 / kDeletePercentage))) {
+    if (rnd.PercentTrue(kDeletePercentage)) {
       gen_key = Key(i);
 
       ASSERT_OK(Delete(gen_key));

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 &&
+      if (thread->rand.OneInOpt(FLAGS_set_options_one_in)) {
-          thread->rand.OneIn(FLAGS_set_options_one_in)) {
         SetOptions(thread);
       }
 
-      if (FLAGS_set_in_place_one_in > 0 &&
+      if (thread->rand.OneInOpt(FLAGS_set_in_place_one_in)) {
-          thread->rand.OneIn(FLAGS_set_in_place_one_in)) {
         options_.inplace_update_support ^= options_.inplace_update_support;
       }
 
       MaybeClearOneColumnFamily(thread);
 
-      if (FLAGS_sync_wal_one_in > 0 &&
+      if (thread->rand.OneInOpt(FLAGS_sync_wal_one_in)) {
-          thread->rand.Uniform(FLAGS_sync_wal_one_in) == 0) {
         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 &&
+      if (thread->rand.OneInOpt(FLAGS_compact_files_one_in)) {
-          thread->rand.Uniform(FLAGS_compact_files_one_in) == 0) {
         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 &&
+      if (thread->rand.OneInOpt(FLAGS_compact_range_one_in)) {
-          thread->rand.Uniform(FLAGS_compact_range_one_in) == 0) {
         TestCompactRange(thread, rand_key, key, column_family);
         if (thread->shared->HasVerificationFailedYet()) {
           break;
@@ -595,8 +590,7 @@ void StressTest::OperateDb(ThreadState* thread) {
       std::vector<int> rand_column_families =
           GenerateColumnFamilies(FLAGS_column_families, rand_column_family);
 
-      if (FLAGS_flush_one_in > 0 &&
+      if (thread->rand.OneInOpt(FLAGS_flush_one_in)) {
-          thread->rand.Uniform(FLAGS_flush_one_in) == 0) {
         FlushOptions flush_opts;
         std::vector<ColumnFamilyHandle*> cfhs;
         std::for_each(
@@ -609,8 +603,7 @@ void StressTest::OperateDb(ThreadState* thread) {
         }
       }
 
-      if (FLAGS_pause_background_one_in > 0 &&
+      if (thread->rand.OneInOpt(FLAGS_pause_background_one_in)) {
-          thread->rand.OneIn(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<int64_t> rand_keys = GenerateKeys(rand_key);
 
-      if (FLAGS_ingest_external_file_one_in > 0 &&
+      if (thread->rand.OneInOpt(FLAGS_ingest_external_file_one_in)) {
-          thread->rand.Uniform(FLAGS_ingest_external_file_one_in) == 0) {
         TestIngestExternalFile(thread, rand_column_families, rand_keys, lock);
       }
 
-      if (FLAGS_backup_one_in > 0 &&
+      if (thread->rand.OneInOpt(FLAGS_backup_one_in)) {
-          thread->rand.Uniform(FLAGS_backup_one_in) == 0) {
         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 &&
+      if (thread->rand.OneInOpt(FLAGS_checkpoint_one_in)) {
-          thread->rand.Uniform(FLAGS_checkpoint_one_in) == 0) {
         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 &&
+      if (thread->rand.OneInOpt(FLAGS_acquire_snapshot_one_in)) {
-          thread->rand.Uniform(FLAGS_acquire_snapshot_one_in) == 0) {
 #ifndef ROCKSDB_LITE
         auto db_impl = reinterpret_cast<DBImpl*>(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<int>(
             std::min(static_cast<uint64_t>(thread->rand.Uniform(4)),

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 (FLAGS_column_families > 1) {
-      if (thread->rand.OneIn(FLAGS_clear_column_family_one_in)) {
+      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));

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                                              \

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<int>(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

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 <cstring>
+#include <vector>
+
+#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<int> counts(range, 0);
+
+      for (int i = 0; i < range * average; ++i) {
+        ++counts.at(r.Uniform(range));
+      }
+      int max_variance = static_cast<int>(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<int>(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<int>(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();
+}