More Makefile Cleanup (facebook#7097)

Summary:
Cleans up some of the dependencies on test code in the Makefile while building tools:
- Moves the test::RandomString, DBBaseTest::RandomString into Random
- Moves the test::RandomHumanReadableString into Random
- Moves the DestroyDir method into file_utils
- Moves the SetupSyncPointsToMockDirectIO into sync_point.
- Moves the FaultInjection Env and FS classes under env

These changes allow all of the tools to build without dependencies on test_util, thereby simplifying the build dependencies.  By moving the FaultInjection code, the dependency in db_stress on different libraries for debug vs release was eliminated.

Tested both release and debug builds via Make and CMake for both static and shared libraries.

More work remains to clean up how the tools are built and remove some unnecessary dependencies.  There is also more work that should be done to get the Makefile and CMake to align in their builds -- what is in the libraries and the sizes of the executables are different.

Pull Request resolved: facebook#7097

Reviewed By: riversand963

Differential Revision: D22463160

Pulled By: pdillinger

fbshipit-source-id: e19462b53324ab3f0b7c72459dbc73165cc382b2

CMakeLists.txt

@@ -751,6 +751,8 @@ set(SOURCES
         utilities/debug.cc
         utilities/env_mirror.cc
         utilities/env_timed.cc
+        utilities/fault_injection_env.cc
+        utilities/fault_injection_fs.cc
         utilities/leveldb_options/leveldb_options.cc
         utilities/memory/memory_util.cc
         utilities/merge_operators/bytesxor.cc
@@ -1172,8 +1174,6 @@ if(WITH_TESTS)
       db/db_test_util.cc
       monitoring/thread_status_updater_debug.cc
       table/mock_table.cc
-      test_util/fault_injection_test_env.cc
-      test_util/fault_injection_test_fs.cc
       utilities/cassandra/test_utils.cc
   )
   enable_testing()

Makefile

@@ -636,18 +636,12 @@ LIBRARY=$(SHARED1)
 TEST_LIBRARY=$(SHARED_TEST_LIBRARY)
 TOOLS_LIBRARY=$(SHARED_TOOLS_LIBRARY)
 STRESS_LIBRARY=$(SHARED_STRESS_LIBRARY)
-ifeq ($(DEBUG_LEVEL),0)
-STRESS_LIBRARY_RUNTIME_DEPS=$(SHARED_TOOLS_LIBRARY)
-else
-STRESS_LIBRARY_RUNTIME_DEPS=$(SHARED_TEST_LIBRARY) $(SHARED_TOOLS_LIBRARY)
-endif
 CLOUD_LIBRARY=$(SHARED_CLOUD_LIBRARY)
 else
 LIBRARY=$(STATIC_LIBRARY)
 TEST_LIBRARY=$(STATIC_TEST_LIBRARY)
 TOOLS_LIBRARY=$(STATIC_TOOLS_LIBRARY)
 STRESS_LIBRARY=$(STATIC_STRESS_LIBRARY)
-STRESS_LIBRARY_RUNTIME_DEPS=
 endif
 
 ROCKSDB_MAJOR = $(shell egrep "ROCKSDB_MAJOR.[0-9]" include/rocksdb/version.h | cut -d ' ' -f 3)
@@ -1166,29 +1160,23 @@ $(STATIC_TEST_LIBRARY): $(TEST_OBJECTS)
 	$(AM_V_AR)rm -f $@ $(SHARED_TEST_LIBRARY)
 	$(AM_V_at)$(AR) $(ARFLAGS) $@ $^
 
-$(STATIC_TOOLS_LIBRARY): $(BENCH_OBJECTS) $(TOOL_OBJECTS) $(TESTUTIL)
+$(STATIC_TOOLS_LIBRARY): $(BENCH_OBJECTS) $(TOOL_OBJECTS)
 	$(AM_V_AR)rm -f $@ $(SHARED_TOOLS_LIBRARY)
 	$(AM_V_at)$(AR) $(ARFLAGS) $@ $^
 
-ifeq ($(DEBUG_LEVEL),0)
-$(STATIC_STRESS_LIBRARY): $(TESTUTIL) $(ANALYZE_OBJECTS) $(STRESS_OBJECTS)
+$(STATIC_STRESS_LIBRARY): $(ANALYZE_OBJECTS) $(STRESS_OBJECTS)
 	$(AM_V_AR)rm -f $@ $(SHARED_STRESS_LIBRARY)
 	$(AM_V_at)$(AR) $(ARFLAGS) $@ $^
-else
-$(STATIC_STRESS_LIBRARY): $(TEST_OBJECTS) $(ANALYZE_OBJECTS) $(STRESS_OBJECTS)
-	$(AM_V_AR)rm -f $@ $(SHARED_STRESS_LIBRARY)
-	$(AM_V_at)$(AR) $(ARFLAGS) $@ $^
-endif
 
 $(SHARED_TEST_LIBRARY): $(TEST_OBJECTS) $(SHARED1)
 	$(AM_V_AR)rm -f $@ $(STATIC_TEST_LIBRARY)
 	$(AM_SHARE)
 
-$(SHARED_TOOLS_LIBRARY): $(TOOL_OBJECTS) $(TESTUTIL) $(SHARED1)
+$(SHARED_TOOLS_LIBRARY): $(TOOL_OBJECTS) $(SHARED1)
 	$(AM_V_AR)rm -f $@ $(STATIC_TOOLS_LIBRARY)
 	$(AM_SHARE)
 
-$(SHARED_STRESS_LIBRARY): $(ANALYZE_OBJECTS) $(STRESS_OBJECTS) $(STRESS_LIBRARY_RUNTIME_DEPS) $(SHARED1)
+$(SHARED_STRESS_LIBRARY): $(ANALYZE_OBJECTS) $(STRESS_OBJECTS) $(SHARED_TOOLS_LIBRARY) $(SHARED1)
 	$(AM_V_AR)rm -f $@ $(STATIC_STRESS_LIBRARY)
 	$(AM_SHARE)
 
@@ -1216,13 +1204,13 @@ cache_bench: $(OBJ_DIR)/cache/cache_bench.o $(LIBRARY)
 persistent_cache_bench: $(OBJ_DIR)/utilities/persistent_cache/persistent_cache_bench.o $(LIBRARY)
 	$(AM_LINK)
 
-memtablerep_bench: $(OBJ_DIR)/memtable/memtablerep_bench.o $(TESTUTIL) $(LIBRARY)
+memtablerep_bench: $(OBJ_DIR)/memtable/memtablerep_bench.o $(LIBRARY)
 	$(AM_LINK)
 
 filter_bench: $(OBJ_DIR)/util/filter_bench.o $(LIBRARY)
 	$(AM_LINK)
 
-db_stress: $(OBJ_DIR)/db_stress_tool/db_stress.o $(STRESS_LIBRARY) $(STRESS_LIBRARY_RUNTIME_DEPS) $(LIBRARY)
+db_stress: $(OBJ_DIR)/db_stress_tool/db_stress.o $(STRESS_LIBRARY) $(TOOLS_LIBRARY) $(LIBRARY)
 	$(AM_LINK)
 
 write_stress: $(OBJ_DIR)/tools/write_stress.o $(LIBRARY)
@@ -1231,7 +1219,7 @@ write_stress: $(OBJ_DIR)/tools/write_stress.o $(LIBRARY)
 db_sanity_test: $(OBJ_DIR)/tools/db_sanity_test.o $(LIBRARY)
 	$(AM_LINK)
 
-db_repl_stress: $(OBJ_DIR)/tools/db_repl_stress.o $(TESTUTIL) $(LIBRARY)
+db_repl_stress: $(OBJ_DIR)/tools/db_repl_stress.o $(LIBRARY)
 	$(AM_LINK)
 
 arena_test: $(OBJ_DIR)/memory/arena_test.o $(TEST_LIBRARY) $(LIBRARY)

TARGETS

@@ -332,6 +332,8 @@ cpp_library(
         "utilities/debug.cc",
         "utilities/env_mirror.cc",
         "utilities/env_timed.cc",
+        "utilities/fault_injection_env.cc",
+        "utilities/fault_injection_fs.cc",
         "utilities/leveldb_options/leveldb_options.cc",
         "utilities/memory/memory_util.cc",
         "utilities/merge_operators/bytesxor.cc",
@@ -385,8 +387,6 @@ cpp_library(
     srcs = [
         "db/db_test_util.cc",
         "table/mock_table.cc",
-        "test_util/fault_injection_test_env.cc",
-        "test_util/fault_injection_test_fs.cc",
         "test_util/testharness.cc",
         "test_util/testutil.cc",
         "tools/block_cache_analyzer/block_cache_trace_analyzer.cc",

db/column_family_test.cc

@@ -22,26 +22,18 @@
 #include "rocksdb/env.h"
 #include "rocksdb/iterator.h"
 #include "rocksdb/utilities/object_registry.h"
-#include "test_util/fault_injection_test_env.h"
 #include "test_util/sync_point.h"
 #include "test_util/testharness.h"
 #include "test_util/testutil.h"
 #include "util/coding.h"
 #include "util/string_util.h"
+#include "utilities/fault_injection_env.h"
 #include "utilities/merge_operators.h"
 
 namespace ROCKSDB_NAMESPACE {
 
 static const int kValueSize = 1000;
 
-namespace {
-std::string RandomString(Random* rnd, int len) {
-  std::string r;
-  test::RandomString(rnd, len, &r);
-  return r;
-}
-}  // anonymous namespace
-
 // counts how many operations were performed
 class EnvCounter : public EnvWrapper {
  public:
@@ -109,11 +101,11 @@ class ColumnFamilyTestBase : public testing::Test {
       // preserves the implementation that was in place when all of the
       // magic values in this file were picked.
       *storage = std::string(kValueSize, ' ');
-      return Slice(*storage);
     } else {
       Random r(k);
-      return test::RandomString(&r, kValueSize, storage);
+      *storage = r.RandomString(kValueSize);
     }
+    return Slice(*storage);
   }
 
   void Build(int base, int n, int flush_every = 0) {
@@ -329,11 +321,11 @@ class ColumnFamilyTestBase : public testing::Test {
       // 10 bytes for key, rest is value
       if (!save) {
         ASSERT_OK(Put(cf, test::RandomKey(&rnd_, 11),
-                      RandomString(&rnd_, key_value_size - 10)));
+                      rnd_.RandomString(key_value_size - 10)));
       } else {
         std::string key = test::RandomKey(&rnd_, 11);
         keys_[cf].insert(key);
-        ASSERT_OK(Put(cf, key, RandomString(&rnd_, key_value_size - 10)));
+        ASSERT_OK(Put(cf, key, rnd_.RandomString(key_value_size - 10)));
       }
     }
     db_->FlushWAL(false);

db/comparator_db_test.cc

@@ -13,6 +13,7 @@
 #include "test_util/testutil.h"
 #include "util/hash.h"
 #include "util/kv_map.h"
+#include "util/random.h"
 #include "util/string_util.h"
 #include "utilities/merge_operators.h"
 
@@ -342,12 +343,12 @@ TEST_P(ComparatorDBTest, SimpleSuffixReverseComparator) {
     std::vector<std::string> source_prefixes;
     // Randomly generate 5 prefixes
     for (int i = 0; i < 5; i++) {
-      source_prefixes.push_back(test::RandomHumanReadableString(&rnd, 8));
+      source_prefixes.push_back(rnd.HumanReadableString(8));
     }
     for (int j = 0; j < 20; j++) {
       int prefix_index = rnd.Uniform(static_cast<int>(source_prefixes.size()));
       std::string key = source_prefixes[prefix_index] +
-                        test::RandomHumanReadableString(&rnd, rnd.Uniform(8));
+                        rnd.HumanReadableString(rnd.Uniform(8));
       source_strings.push_back(key);
     }
 

db/corruption_test.cc

@@ -9,13 +9,13 @@
 
 #ifndef ROCKSDB_LITE
 
-#include "rocksdb/db.h"
-
 #include <errno.h>
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <sys/types.h>
+
 #include <cinttypes>
+
 #include "db/db_impl/db_impl.h"
 #include "db/db_test_util.h"
 #include "db/log_format.h"
@@ -24,13 +24,15 @@
 #include "file/filename.h"
 #include "rocksdb/cache.h"
 #include "rocksdb/convenience.h"
+#include "rocksdb/db.h"
 #include "rocksdb/env.h"
 #include "rocksdb/table.h"
 #include "rocksdb/write_batch.h"
 #include "table/block_based/block_based_table_builder.h"
 #include "table/meta_blocks.h"
 #include "test_util/testharness.h"
 #include "test_util/testutil.h"
+#include "util/random.h"
 #include "util/string_util.h"
 
 namespace ROCKSDB_NAMESPACE {
@@ -219,11 +221,11 @@ class CorruptionTest : public testing::Test {
       // preserves the implementation that was in place when all of the
       // magic values in this file were picked.
       *storage = std::string(kValueSize, ' ');
-      return Slice(*storage);
     } else {
       Random r(k);
-      return test::RandomString(&r, kValueSize, storage);
+      *storage = r.RandomString(kValueSize);
     }
+    return Slice(*storage);
   }
 };
 

db/db_basic_test.cc

@@ -16,10 +16,11 @@
 #include "rocksdb/utilities/debug.h"
 #include "table/block_based/block_based_table_reader.h"
 #include "table/block_based/block_builder.h"
-#include "test_util/fault_injection_test_env.h"
 #if !defined(ROCKSDB_LITE)
 #include "test_util/sync_point.h"
 #endif
+#include "util/random.h"
+#include "utilities/fault_injection_env.h"
 #include "utilities/merge_operators.h"
 #include "utilities/merge_operators/string_append/stringappend.h"
 
@@ -2040,7 +2041,7 @@ TEST_F(DBBasicTest, MultiGetIOBufferOverrun) {
   for (int i = 0; i < 100; ++i) {
     // Make the value compressible. A purely random string doesn't compress
     // and the resultant data block will not be compressed
-    std::string value(RandomString(&rnd, 128) + zero_str);
+    std::string value(rnd.RandomString(128) + zero_str);
     assert(Put(Key(i), value) == Status::OK());
   }
   Flush();
@@ -2430,7 +2431,7 @@ class DBBasicTestMultiGet : public DBTestBase {
       for (int i = 0; i < 100; ++i) {
         // Make the value compressible. A purely random string doesn't compress
         // and the resultant data block will not be compressed
-        values_.emplace_back(RandomString(&rnd, 128) + zero_str);
+        values_.emplace_back(rnd.RandomString(128) + zero_str);
         assert(((num_cfs == 1) ? Put(Key(i), values_[i])
                                : Put(cf, Key(i), values_[i])) == Status::OK());
       }
@@ -2442,7 +2443,7 @@ class DBBasicTestMultiGet : public DBTestBase {
 
       for (int i = 0; i < 100; ++i) {
         // block cannot gain space by compression
-        uncompressable_values_.emplace_back(RandomString(&rnd, 256) + '\0');
+        uncompressable_values_.emplace_back(rnd.RandomString(256) + '\0');
         std::string tmp_key = "a" + Key(i);
         assert(((num_cfs == 1) ? Put(tmp_key, uncompressable_values_[i])
                                : Put(cf, tmp_key, uncompressable_values_[i])) ==
@@ -3210,7 +3211,7 @@ TEST_F(DBBasicTest, PointLookupDeadline) {
     Random rnd(301);
     for (int i = 0; i < 400; ++i) {
       std::string key = "k" + ToString(i);
-      Put(key, RandomString(&rnd, 100));
+      Put(key, rnd.RandomString(100));
     }
     Flush();
 

db/db_block_cache_test.cc

@@ -7,10 +7,12 @@
 // 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 <cstdlib>
+
 #include "cache/lru_cache.h"
 #include "db/db_test_util.h"
 #include "port/stack_trace.h"
 #include "util/compression.h"
+#include "util/random.h"
 
 namespace ROCKSDB_NAMESPACE {
 
@@ -764,7 +766,7 @@ TEST_F(DBBlockCacheTest, CompressedCache) {
     std::string str;
     for (int i = 0; i < num_iter; i++) {
       if (i % 4 == 0) {  // high compression ratio
-        str = RandomString(&rnd, 1000);
+        str = rnd.RandomString(1000);
       }
       values.push_back(str);
       ASSERT_OK(Put(1, Key(i), values[i]));
@@ -851,7 +853,7 @@ TEST_F(DBBlockCacheTest, CacheCompressionDict) {
     for (int i = 0; i < kNumFiles; ++i) {
       ASSERT_EQ(i, NumTableFilesAtLevel(0, 0));
       for (int j = 0; j < kNumEntriesPerFile; ++j) {
-        std::string value = RandomString(&rnd, kNumBytesPerEntry);
+        std::string value = rnd.RandomString(kNumBytesPerEntry);
         ASSERT_OK(Put(Key(j * kNumFiles + i), value.c_str()));
       }
       ASSERT_OK(Flush());