Fix remaining uses of "backupable" (#9792)

Summary:
Various renaming and fixes to get rid of remaining uses of
"backupable" which is terminology leftover from the original, flawed
design of BackupableDB. Now any DB can be backed up, using BackupEngine.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/9792

Test Plan: CI

Reviewed By: ajkr

Differential Revision: D35334386

Pulled By: pdillinger

fbshipit-source-id: 2108a42b4575c8cccdfd791c549aae93ec2f3329

.travis.yml

@@ -239,10 +239,10 @@ script:
       OPT=-DTRAVIS LIB_MODE=shared V=1 ROCKSDBTESTS_PLATFORM_DEPENDENT=only make -j$MK_PARALLEL all_but_some_tests check_some
       ;;
     1)
-      OPT=-DTRAVIS LIB_MODE=shared V=1 ROCKSDBTESTS_PLATFORM_DEPENDENT=exclude ROCKSDBTESTS_END=backupable_db_test make -j$MK_PARALLEL check_some
+      OPT=-DTRAVIS LIB_MODE=shared V=1 ROCKSDBTESTS_PLATFORM_DEPENDENT=exclude ROCKSDBTESTS_END=backup_engine_test make -j$MK_PARALLEL check_some
       ;;
     2)
-      OPT="-DTRAVIS -DROCKSDB_NAMESPACE=alternative_rocksdb_ns" LIB_MODE=shared V=1 make -j$MK_PARALLEL tools && OPT="-DTRAVIS -DROCKSDB_NAMESPACE=alternative_rocksdb_ns" LIB_MODE=shared V=1 ROCKSDBTESTS_PLATFORM_DEPENDENT=exclude ROCKSDBTESTS_START=backupable_db_test ROCKSDBTESTS_END=db_universal_compaction_test make -j$MK_PARALLEL check_some
+      OPT="-DTRAVIS -DROCKSDB_NAMESPACE=alternative_rocksdb_ns" LIB_MODE=shared V=1 make -j$MK_PARALLEL tools && OPT="-DTRAVIS -DROCKSDB_NAMESPACE=alternative_rocksdb_ns" LIB_MODE=shared V=1 ROCKSDBTESTS_PLATFORM_DEPENDENT=exclude ROCKSDBTESTS_START=backup_engine_test ROCKSDBTESTS_END=db_universal_compaction_test make -j$MK_PARALLEL check_some
       ;;
     3)
       OPT=-DTRAVIS LIB_MODE=shared V=1 ROCKSDBTESTS_PLATFORM_DEPENDENT=exclude ROCKSDBTESTS_START=db_universal_compaction_test ROCKSDBTESTS_END=table_properties_collector_test make -j$MK_PARALLEL check_some

CMakeLists.txt

@@ -849,7 +849,7 @@ set(SOURCES
         util/thread_local.cc
         util/threadpool_imp.cc
         util/xxhash.cc
-        utilities/backupable/backupable_db.cc
+        utilities/backup/backup_engine.cc
         utilities/blob_db/blob_compaction_filter.cc
         utilities/blob_db/blob_db.cc
         utilities/blob_db/blob_db_impl.cc
@@ -1315,7 +1315,7 @@ if(WITH_TESTS)
         util/thread_list_test.cc
         util/thread_local_test.cc
         util/work_queue_test.cc
-        utilities/backupable/backupable_db_test.cc
+        utilities/backup/backup_engine_test.cc
         utilities/blob_db/blob_db_test.cc
         utilities/cassandra/cassandra_functional_test.cc
         utilities/cassandra/cassandra_format_test.cc

Makefile

@@ -1551,7 +1551,7 @@ perf_context_test: $(OBJ_DIR)/db/perf_context_test.o $(TEST_LIBRARY) $(LIBRARY)
 prefix_test: $(OBJ_DIR)/db/prefix_test.o $(TEST_LIBRARY) $(LIBRARY)
 	$(AM_LINK)
 
-backupable_db_test: $(OBJ_DIR)/utilities/backupable/backupable_db_test.o $(TEST_LIBRARY) $(LIBRARY)
+backup_engine_test: $(OBJ_DIR)/utilities/backup/backup_engine_test.o $(TEST_LIBRARY) $(LIBRARY)
 	$(AM_LINK)
 
 checkpoint_test: $(OBJ_DIR)/utilities/checkpoint/checkpoint_test.o $(TEST_LIBRARY) $(LIBRARY)

ROCKSDB_LITE.md

@@ -4,7 +4,7 @@ RocksDBLite is a project focused on mobile use cases, which don't need a lot of
 
 Some examples of the features disabled by ROCKSDB_LITE:
 * compiled-in support for LDB tool
-* No backupable DB
+* No backup engine
 * No support for replication (which we provide in form of TransactionalIterator)
 * No advanced monitoring tools
 * No special-purpose memtables that are highly optimized for specific use cases

TARGETS

@@ -245,7 +245,7 @@ cpp_library_wrapper(name="rocksdb_lib", srcs=[
         "util/thread_local.cc",
         "util/threadpool_imp.cc",
         "util/xxhash.cc",
-        "utilities/backupable/backupable_db.cc",
+        "utilities/backup/backup_engine.cc",
         "utilities/blob_db/blob_compaction_filter.cc",
         "utilities/blob_db/blob_db.cc",
         "utilities/blob_db/blob_db_impl.cc",
@@ -563,7 +563,7 @@ cpp_library_wrapper(name="rocksdb_whole_archive_lib", srcs=[
         "util/thread_local.cc",
         "util/threadpool_imp.cc",
         "util/xxhash.cc",
-        "utilities/backupable/backupable_db.cc",
+        "utilities/backup/backup_engine.cc",
         "utilities/blob_db/blob_compaction_filter.cc",
         "utilities/blob_db/blob_db.cc",
         "utilities/blob_db/blob_db_impl.cc",
@@ -4768,8 +4768,8 @@ cpp_unittest_wrapper(name="autovector_test",
             extra_compiler_flags=[])
 
 
-cpp_unittest_wrapper(name="backupable_db_test",
-            srcs=["utilities/backupable/backupable_db_test.cc"],
+cpp_unittest_wrapper(name="backup_engine_test",
+            srcs=["utilities/backup/backup_engine_test.cc"],
             deps=[":rocksdb_test_lib"],
             extra_compiler_flags=[])
 

build_tools/run_ci_db_test.ps1

@@ -42,7 +42,7 @@ $RunOnly.Add("c_test") | Out-Null
 $RunOnly.Add("compact_on_deletion_collector_test") | Out-Null
 $RunOnly.Add("merge_test") | Out-Null
 $RunOnly.Add("stringappend_test") | Out-Null # Apparently incorrectly written
-$RunOnly.Add("backupable_db_test") | Out-Null # Disabled
+$RunOnly.Add("backup_engine_test") | Out-Null # Disabled
 $RunOnly.Add("timer_queue_test") | Out-Null # Not a gtest
 
 if($RunAll -and $SuiteRun -ne "") {
@@ -491,5 +491,3 @@ if(!$script:success) {
  }
 
  exit 0
-
-

db/c_test.c

@@ -2448,7 +2448,7 @@ int main(int argc, char** argv) {
     rocksdb_fifo_compaction_options_destroy(fco);
   }
 
-  StartPhase("backupable_db_option");
+  StartPhase("backup_engine_option");
   {
     rocksdb_backup_engine_options_t* bdo;
     bdo = rocksdb_backup_engine_options_create("path");

db_stress_tool/db_stress_test_base.cc

@@ -21,7 +21,7 @@
 #include "rocksdb/utilities/object_registry.h"
 #include "test_util/testutil.h"
 #include "util/cast_util.h"
-#include "utilities/backupable/backupable_db_impl.h"
+#include "utilities/backup/backup_engine_impl.h"
 #include "utilities/fault_injection_fs.h"
 #include "utilities/fault_injection_secondary_cache.h"
 

java/CMakeLists.txt

@@ -7,7 +7,7 @@ endif()
 set(CMAKE_JAVA_COMPILE_FLAGS -source 7)
 
 set(JNI_NATIVE_SOURCES
-        rocksjni/backupablejni.cc
+        rocksjni/backup_engine_options.cc
         rocksjni/backupenginejni.cc
         rocksjni/cassandra_compactionfilterjni.cc
         rocksjni/cassandra_value_operator.cc

java/rocksjni/portal.h

@@ -3468,7 +3468,7 @@ class BackupEngineJni
                                 BackupEngineJni> {
  public:
   /**
-   * Get the Java Class org.rocksdb.BackupableEngine
+   * Get the Java Class org.rocksdb.BackupEngine
    *
    * @param env A pointer to the Java environment
    *

java/src/main/java/org/rocksdb/BackupEngine.java

@@ -226,8 +226,8 @@ public class BackupEngine extends RocksObject implements AutoCloseable {
         restoreOptions.nativeHandle_);
   }
 
-  private native static long open(final long env,
-      final long backupableDbOptions) throws RocksDBException;
+  private native static long open(final long env, final long backupEngineOptions)
+      throws RocksDBException;
 
   private native void createNewBackup(final long handle, final long dbHandle,
       final boolean flushBeforeBackup) throws RocksDBException;

java/src/main/java/org/rocksdb/BackupEngineOptions.java

@@ -8,8 +8,8 @@ package org.rocksdb;
 import java.io.File;
 
 /**
- * <p>BackupEngineOptions to control the behavior of a backupable database.
- * It will be used during the creation of a {@link org.rocksdb.BackupEngine}.
+ * <p>BackupEngineOptions controls the behavior of a
+ * {@link org.rocksdb.BackupEngine}.
  * </p>
  * <p>Note that dispose() must be called before an Options instance
  * become out-of-scope to release the allocated memory in c++.</p>

java/src/test/java/org/rocksdb/BackupEngineOptionsTest.java

@@ -30,41 +30,36 @@ public class BackupEngineOptionsTest {
 
   @Test
   public void backupDir() {
-    try (final BackupEngineOptions backupableDBOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
-      assertThat(backupableDBOptions.backupDir()).
-          isEqualTo(ARBITRARY_PATH);
+    try (final BackupEngineOptions backupEngineOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
+      assertThat(backupEngineOptions.backupDir()).isEqualTo(ARBITRARY_PATH);
     }
   }
 
   @Test
   public void env() {
-    try (final BackupEngineOptions backupableDBOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
-      assertThat(backupableDBOptions.backupEnv()).
-          isNull();
+    try (final BackupEngineOptions backupEngineOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
+      assertThat(backupEngineOptions.backupEnv()).isNull();
 
       try(final Env env = new RocksMemEnv(Env.getDefault())) {
-        backupableDBOptions.setBackupEnv(env);
-        assertThat(backupableDBOptions.backupEnv())
-            .isEqualTo(env);
+        backupEngineOptions.setBackupEnv(env);
+        assertThat(backupEngineOptions.backupEnv()).isEqualTo(env);
       }
     }
   }
 
   @Test
   public void shareTableFiles() {
-    try (final BackupEngineOptions backupableDBOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
+    try (final BackupEngineOptions backupEngineOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
       final boolean value = rand.nextBoolean();
-      backupableDBOptions.setShareTableFiles(value);
-      assertThat(backupableDBOptions.shareTableFiles()).
-          isEqualTo(value);
+      backupEngineOptions.setShareTableFiles(value);
+      assertThat(backupEngineOptions.shareTableFiles()).isEqualTo(value);
     }
   }
 
   @Test
   public void infoLog() {
-    try (final BackupEngineOptions backupableDBOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
-      assertThat(backupableDBOptions.infoLog()).
-          isNull();
+    try (final BackupEngineOptions backupEngineOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
+      assertThat(backupEngineOptions.infoLog()).isNull();
 
       try(final Options options = new Options();
           final Logger logger = new Logger(options){
@@ -73,127 +68,113 @@ public class BackupEngineOptionsTest {
 
             }
           }) {
-        backupableDBOptions.setInfoLog(logger);
-        assertThat(backupableDBOptions.infoLog())
-            .isEqualTo(logger);
+        backupEngineOptions.setInfoLog(logger);
+        assertThat(backupEngineOptions.infoLog()).isEqualTo(logger);
       }
     }
   }
 
   @Test
   public void sync() {
-    try (final BackupEngineOptions backupableDBOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
+    try (final BackupEngineOptions backupEngineOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
       final boolean value = rand.nextBoolean();
-      backupableDBOptions.setSync(value);
-      assertThat(backupableDBOptions.sync()).isEqualTo(value);
+      backupEngineOptions.setSync(value);
+      assertThat(backupEngineOptions.sync()).isEqualTo(value);
     }
   }
 
   @Test
   public void destroyOldData() {
-    try (final BackupEngineOptions backupableDBOptions = new BackupEngineOptions(ARBITRARY_PATH);) {
+    try (final BackupEngineOptions backupEngineOptions = new BackupEngineOptions(ARBITRARY_PATH);) {
       final boolean value = rand.nextBoolean();
-      backupableDBOptions.setDestroyOldData(value);
-      assertThat(backupableDBOptions.destroyOldData()).
-          isEqualTo(value);
+      backupEngineOptions.setDestroyOldData(value);
+      assertThat(backupEngineOptions.destroyOldData()).isEqualTo(value);
     }
   }
 
   @Test
   public void backupLogFiles() {
-    try (final BackupEngineOptions backupableDBOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
+    try (final BackupEngineOptions backupEngineOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
       final boolean value = rand.nextBoolean();
-      backupableDBOptions.setBackupLogFiles(value);
-      assertThat(backupableDBOptions.backupLogFiles()).
-          isEqualTo(value);
+      backupEngineOptions.setBackupLogFiles(value);
+      assertThat(backupEngineOptions.backupLogFiles()).isEqualTo(value);
     }
   }
 
   @Test
   public void backupRateLimit() {
-    try (final BackupEngineOptions backupableDBOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
+    try (final BackupEngineOptions backupEngineOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
       final long value = Math.abs(rand.nextLong());
-      backupableDBOptions.setBackupRateLimit(value);
-      assertThat(backupableDBOptions.backupRateLimit()).
-          isEqualTo(value);
+      backupEngineOptions.setBackupRateLimit(value);
+      assertThat(backupEngineOptions.backupRateLimit()).isEqualTo(value);
       // negative will be mapped to 0
-      backupableDBOptions.setBackupRateLimit(-1);
-      assertThat(backupableDBOptions.backupRateLimit()).
-          isEqualTo(0);
+      backupEngineOptions.setBackupRateLimit(-1);
+      assertThat(backupEngineOptions.backupRateLimit()).isEqualTo(0);
     }
   }
 
   @Test
   public void backupRateLimiter() {
-    try (final BackupEngineOptions backupableDBOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
-      assertThat(backupableDBOptions.backupEnv()).
-          isNull();
+    try (final BackupEngineOptions backupEngineOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
+      assertThat(backupEngineOptions.backupEnv()).isNull();
 
       try(final RateLimiter backupRateLimiter =
               new RateLimiter(999)) {
-        backupableDBOptions.setBackupRateLimiter(backupRateLimiter);
-        assertThat(backupableDBOptions.backupRateLimiter())
-            .isEqualTo(backupRateLimiter);
+        backupEngineOptions.setBackupRateLimiter(backupRateLimiter);
+        assertThat(backupEngineOptions.backupRateLimiter()).isEqualTo(backupRateLimiter);
       }
     }
   }
 
   @Test
   public void restoreRateLimit() {
-    try (final BackupEngineOptions backupableDBOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
+    try (final BackupEngineOptions backupEngineOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
       final long value = Math.abs(rand.nextLong());
-      backupableDBOptions.setRestoreRateLimit(value);
-      assertThat(backupableDBOptions.restoreRateLimit()).
-          isEqualTo(value);
+      backupEngineOptions.setRestoreRateLimit(value);
+      assertThat(backupEngineOptions.restoreRateLimit()).isEqualTo(value);
       // negative will be mapped to 0
-      backupableDBOptions.setRestoreRateLimit(-1);
-      assertThat(backupableDBOptions.restoreRateLimit()).
-          isEqualTo(0);
+      backupEngineOptions.setRestoreRateLimit(-1);
+      assertThat(backupEngineOptions.restoreRateLimit()).isEqualTo(0);
     }
   }
 
   @Test
   public void restoreRateLimiter() {
-    try (final BackupEngineOptions backupableDBOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
-      assertThat(backupableDBOptions.backupEnv()).
-          isNull();
+    try (final BackupEngineOptions backupEngineOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
+      assertThat(backupEngineOptions.backupEnv()).isNull();
 
       try(final RateLimiter restoreRateLimiter =
               new RateLimiter(911)) {
-        backupableDBOptions.setRestoreRateLimiter(restoreRateLimiter);
-        assertThat(backupableDBOptions.restoreRateLimiter())
-            .isEqualTo(restoreRateLimiter);
+        backupEngineOptions.setRestoreRateLimiter(restoreRateLimiter);
+        assertThat(backupEngineOptions.restoreRateLimiter()).isEqualTo(restoreRateLimiter);
       }
     }
   }
 
   @Test
   public void shareFilesWithChecksum() {
-    try (final BackupEngineOptions backupableDBOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
+    try (final BackupEngineOptions backupEngineOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
       boolean value = rand.nextBoolean();
-      backupableDBOptions.setShareFilesWithChecksum(value);
-      assertThat(backupableDBOptions.shareFilesWithChecksum()).
-          isEqualTo(value);
+      backupEngineOptions.setShareFilesWithChecksum(value);
+      assertThat(backupEngineOptions.shareFilesWithChecksum()).isEqualTo(value);
     }
   }
 
   @Test
   public void maxBackgroundOperations() {
-    try (final BackupEngineOptions backupableDBOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
+    try (final BackupEngineOptions backupEngineOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
       final int value = rand.nextInt();
-      backupableDBOptions.setMaxBackgroundOperations(value);
-      assertThat(backupableDBOptions.maxBackgroundOperations()).
-          isEqualTo(value);
+      backupEngineOptions.setMaxBackgroundOperations(value);
+      assertThat(backupEngineOptions.maxBackgroundOperations()).isEqualTo(value);
     }
   }
 
   @Test
   public void callbackTriggerIntervalSize() {
-    try (final BackupEngineOptions backupableDBOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
+    try (final BackupEngineOptions backupEngineOptions = new BackupEngineOptions(ARBITRARY_PATH)) {
       final long value = rand.nextLong();
-      backupableDBOptions.setCallbackTriggerIntervalSize(value);
-      assertThat(backupableDBOptions.callbackTriggerIntervalSize()).
-          isEqualTo(value);
+      backupEngineOptions.setCallbackTriggerIntervalSize(value);
+      assertThat(backupEngineOptions.callbackTriggerIntervalSize()).isEqualTo(value);
     }
   }
 
@@ -311,9 +292,9 @@ public class BackupEngineOptionsTest {
   }
 
   private BackupEngineOptions setupUninitializedBackupEngineOptions(ExpectedException exception) {
-    final BackupEngineOptions backupableDBOptions = new BackupEngineOptions(ARBITRARY_PATH);
-    backupableDBOptions.close();
+    final BackupEngineOptions backupEngineOptions = new BackupEngineOptions(ARBITRARY_PATH);
+    backupEngineOptions.close();
     exception.expect(AssertionError.class);
-    return backupableDBOptions;
+    return backupEngineOptions;
   }
 }

src.mk

@@ -232,7 +232,7 @@ LIB_SOURCES =                                                   \
   util/thread_local.cc                                          \
   util/threadpool_imp.cc                                        \
   util/xxhash.cc                                                \
-  utilities/backupable/backupable_db.cc                         \
+  utilities/backup/backup_engine.cc                             \
   utilities/blob_db/blob_compaction_filter.cc                   \
   utilities/blob_db/blob_db.cc                                  \
   utilities/blob_db/blob_db_impl.cc                             \
@@ -560,7 +560,7 @@ TEST_MAIN_SOURCES =                                                     \
   util/thread_list_test.cc                                              \
   util/thread_local_test.cc                                             \
   util/work_queue_test.cc                                               \
-  utilities/backupable/backupable_db_test.cc                            \
+  utilities/backup/backup_engine_test.cc                                \
   utilities/blob_db/blob_db_test.cc                                     \
   utilities/cassandra/cassandra_format_test.cc                          \
   utilities/cassandra/cassandra_functional_test.cc                      \
@@ -598,7 +598,7 @@ MICROBENCH_SOURCES =                                          \
 
 JNI_NATIVE_SOURCES =                                          \
   java/rocksjni/backupenginejni.cc                            \
-  java/rocksjni/backupablejni.cc                              \
+  java/rocksjni/backup_engine_options.cc                      \
   java/rocksjni/checkpoint.cc                                 \
   java/rocksjni/clock_cache.cc                                \
   java/rocksjni/cache.cc                                      \

tools/ldb_cmd.cc

@@ -3296,13 +3296,14 @@ void RepairCommand::DoCommand() {
 
 // ----------------------------------------------------------------------------
 
-const std::string BackupableCommand::ARG_NUM_THREADS = "num_threads";
-const std::string BackupableCommand::ARG_BACKUP_ENV_URI = "backup_env_uri";
-const std::string BackupableCommand::ARG_BACKUP_FS_URI = "backup_fs_uri";
-const std::string BackupableCommand::ARG_BACKUP_DIR = "backup_dir";
-const std::string BackupableCommand::ARG_STDERR_LOG_LEVEL = "stderr_log_level";
-
-BackupableCommand::BackupableCommand(
+const std::string BackupEngineCommand::ARG_NUM_THREADS = "num_threads";
+const std::string BackupEngineCommand::ARG_BACKUP_ENV_URI = "backup_env_uri";
+const std::string BackupEngineCommand::ARG_BACKUP_FS_URI = "backup_fs_uri";
+const std::string BackupEngineCommand::ARG_BACKUP_DIR = "backup_dir";
+const std::string BackupEngineCommand::ARG_STDERR_LOG_LEVEL =
+    "stderr_log_level";
+
+BackupEngineCommand::BackupEngineCommand(
     const std::vector<std::string>& /*params*/,
     const std::map<std::string, std::string>& options,
     const std::vector<std::string>& flags)
@@ -3351,7 +3352,7 @@ BackupableCommand::BackupableCommand(
   }
 }
 
-void BackupableCommand::Help(const std::string& name, std::string& ret) {
+void BackupEngineCommand::Help(const std::string& name, std::string& ret) {
   ret.append("  ");
   ret.append(name);
   ret.append(" [--" + ARG_BACKUP_ENV_URI + " | --" + ARG_BACKUP_FS_URI + "] ");
@@ -3366,10 +3367,10 @@ void BackupableCommand::Help(const std::string& name, std::string& ret) {
 BackupCommand::BackupCommand(const std::vector<std::string>& params,
                              const std::map<std::string, std::string>& options,
                              const std::vector<std::string>& flags)
-    : BackupableCommand(params, options, flags) {}
+    : BackupEngineCommand(params, options, flags) {}
 
 void BackupCommand::Help(std::string& ret) {
-  BackupableCommand::Help(Name(), ret);
+  BackupEngineCommand::Help(Name(), ret);
 }
 
 void BackupCommand::DoCommand() {
@@ -3419,10 +3420,10 @@ RestoreCommand::RestoreCommand(
     const std::vector<std::string>& params,
     const std::map<std::string, std::string>& options,
     const std::vector<std::string>& flags)
-    : BackupableCommand(params, options, flags) {}
+    : BackupEngineCommand(params, options, flags) {}
 
 void RestoreCommand::Help(std::string& ret) {
-  BackupableCommand::Help(Name(), ret);
+  BackupEngineCommand::Help(Name(), ret);
 }
 
 void RestoreCommand::DoCommand() {

tools/ldb_cmd_impl.h

@@ -579,11 +579,11 @@ class RepairCommand : public LDBCommand {
   static const std::string ARG_VERBOSE;
 };
 
-class BackupableCommand : public LDBCommand {
+class BackupEngineCommand : public LDBCommand {
  public:
-  BackupableCommand(const std::vector<std::string>& params,
-                    const std::map<std::string, std::string>& options,
-                    const std::vector<std::string>& flags);
+  BackupEngineCommand(const std::vector<std::string>& params,
+                      const std::map<std::string, std::string>& options,
+                      const std::vector<std::string>& flags);
 
  protected:
   static void Help(const std::string& name, std::string& ret);
@@ -602,7 +602,7 @@ class BackupableCommand : public LDBCommand {
   static const std::string ARG_STDERR_LOG_LEVEL;
 };
 
-class BackupCommand : public BackupableCommand {
+class BackupCommand : public BackupEngineCommand {
  public:
   static std::string Name() { return "backup"; }
   BackupCommand(const std::vector<std::string>& params,
@@ -612,7 +612,7 @@ class BackupCommand : public BackupableCommand {
   static void Help(std::string& ret);
 };
 
-class RestoreCommand : public BackupableCommand {
+class RestoreCommand : public BackupEngineCommand {
  public:
   static std::string Name() { return "restore"; }
   RestoreCommand(const std::vector<std::string>& params,

utilities/backup/backup_engine.cc

@@ -49,7 +49,7 @@
 #include "util/crc32c.h"
 #include "util/math.h"
 #include "util/string_util.h"
-#include "utilities/backupable/backupable_db_impl.h"
+#include "utilities/backup/backup_engine_impl.h"
 #include "utilities/checkpoint/checkpoint_impl.h"
 
 namespace ROCKSDB_NAMESPACE {
@@ -126,6 +126,7 @@ void BackupEngineOptions::Dump(Logger* logger) const {
                  max_background_operations);
 }
 
+namespace {
 // -------- BackupEngineImpl class ---------
 class BackupEngineImpl {
  public:
@@ -408,8 +409,9 @@ class BackupEngineImpl {
 
     std::shared_ptr<FileInfo> GetFile(const std::string& filename) const {
       auto it = file_infos_->find(filename);
-      if (it == file_infos_->end())
+      if (it == file_infos_->end()) {
         return nullptr;
+      }
       return it->second;
     }
 
@@ -944,6 +946,7 @@ class BackupEngineImplThreadSafe : public BackupEngine,
   mutable port::RWMutex mutex_;
   BackupEngineImpl impl_;
 };
+}  // namespace
 
 IOStatus BackupEngine::Open(const BackupEngineOptions& options, Env* env,
                             BackupEngine** backup_engine_ptr) {
@@ -958,6 +961,7 @@ IOStatus BackupEngine::Open(const BackupEngineOptions& options, Env* env,
   return IOStatus::OK();
 }
 
+namespace {
 BackupEngineImpl::BackupEngineImpl(const BackupEngineOptions& options,
                                    Env* db_env, bool read_only)
     : initialized_(false),
@@ -2697,8 +2701,6 @@ IOStatus BackupEngineImpl::BackupMeta::Delete(bool delete_meta) {
 }
 
 // Constants for backup meta file schema (see LoadFromFile)
-namespace {
-
 const std::string kSchemaVersionPrefix{"schema_version "};
 const std::string kFooterMarker{"// FOOTER"};
 
@@ -2716,7 +2718,6 @@ const std::string kTemperatureFieldName{"temp"};
 // to indicate all file names have had spaces and special characters
 // escaped using a URI percent encoding.
 const std::string kNonIgnorableFieldPrefix{"ni::"};
-}  // namespace
 
 // Each backup meta file is of the format (schema version 1):
 //----------------------------------------------------------
@@ -3029,13 +3030,11 @@ IOStatus BackupEngineImpl::BackupMeta::LoadFromFile(
   return IOStatus::OK();
 }
 
-namespace {
 const std::vector<std::string> minor_version_strings{
     "",  // invalid major version 0
     "",  // implicit major version 1
     "2.0",
 };
-}  // namespace
 
 IOStatus BackupEngineImpl::BackupMeta::StoreToFile(
     bool sync, int schema_version,
@@ -3133,6 +3132,7 @@ IOStatus BackupEngineImpl::BackupMeta::StoreToFile(
   }
   return io_s;
 }
+}  // namespace
 
 IOStatus BackupEngineReadOnly::Open(const BackupEngineOptions& options,
                                     Env* env,

utilities/backup/backup_engine_test.cc

@@ -9,6 +9,8 @@
 
 #if !defined(ROCKSDB_LITE) && !defined(OS_WIN)
 
+#include "rocksdb/utilities/backup_engine.h"
+
 #include <algorithm>
 #include <array>
 #include <atomic>
@@ -34,7 +36,6 @@
 #include "rocksdb/statistics.h"
 #include "rocksdb/transaction_log.h"
 #include "rocksdb/types.h"
-#include "rocksdb/utilities/backup_engine.h"
 #include "rocksdb/utilities/options_util.h"
 #include "rocksdb/utilities/stackable_db.h"
 #include "test_util/sync_point.h"
@@ -46,7 +47,7 @@
 #include "util/rate_limiter.h"
 #include "util/stderr_logger.h"
 #include "util/string_util.h"
-#include "utilities/backupable/backupable_db_impl.h"
+#include "utilities/backup/backup_engine_impl.h"
 
 namespace ROCKSDB_NAMESPACE {
 
@@ -94,37 +95,6 @@ class DummyDB : public StackableDB {
 
   ColumnFamilyHandle* DefaultColumnFamily() const override { return nullptr; }
 
-  class DummyLogFile : public LogFile {
-   public:
-    /* implicit */
-     DummyLogFile(const std::string& path, bool alive = true)
-         : path_(path), alive_(alive) {}
-
-     std::string PathName() const override { return path_; }
-
-     uint64_t LogNumber() const override {
-       // what business do you have calling this method?
-       ADD_FAILURE();
-       return 0;
-     }
-
-     WalFileType Type() const override {
-       return alive_ ? kAliveLogFile : kArchivedLogFile;
-     }
-
-     SequenceNumber StartSequence() const override {
-       // this seqnum guarantees the dummy file will be included in the backup
-       // as long as it is alive.
-       return kMaxSequenceNumber;
-     }
-
-     uint64_t SizeFileBytes() const override { return 0; }
-
-    private:
-     std::string path_;
-     bool alive_;
-  }; // DummyLogFile
-
   Status GetLiveFilesStorageInfo(
       const LiveFilesStorageInfoOptions& opts,
       std::vector<LiveFileStorageInfo>* files) override {
@@ -231,8 +201,8 @@ class TestFs : public FileSystemWrapper {
                            IODebugContext* dbg) override {
     MutexLock l(&mutex_);
     written_files_.push_back(f);
-    if (limit_written_files_ <= 0) {
-      return IOStatus::NotSupported("Sorry, can't do this");
+    if (limit_written_files_ == 0) {
+      return IOStatus::NotSupported("Limit on written files reached");
     }
     limit_written_files_--;
     IOStatus s = FileSystemWrapper::NewWritableFile(f, file_opts, r, dbg);
@@ -405,6 +375,7 @@ class TestFs : public FileSystemWrapper {
   int num_seq_readers() { return num_seq_readers_; }
   int num_direct_seq_readers() { return num_direct_seq_readers_; }
   int num_writers() { return num_writers_; }
+  // FIXME(?): unused
   int num_direct_writers() { return num_direct_writers_; }
 
  private: