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: facebook/rocksdb#9792 Test Plan: CI Reviewed By: ajkr Differential Revision: D35334386 Pulled By: pdillinger fbshipit-source-id: 2108a42b4575c8cccdfd791c549aae93ec2f3329
mrambacher · Apr 5, 2022 · 6534c6d · 6534c6d
1 parent 9cd47ce
commit 6534c6d
Show file tree

Hide file tree

Showing 20 changed files with 106 additions and 155 deletions.
diff --git a/.travis.yml b/.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

diff --git a/CMakeLists.txt b/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

diff --git a/Makefile b/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)

diff --git a/ROCKSDB_LITE.md b/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

diff --git a/TARGETS b/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=[])
 

diff --git a/build_tools/run_ci_db_test.ps1 b/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
-
-
diff --git a/db/c_test.c b/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");

diff --git a/db_stress_tool/db_stress_test_base.cc b/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"
 

diff --git a/java/CMakeLists.txt b/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

diff --git a/java/rocksjni/backupablejni.cc → java/rocksjni/backup_engine_options.cc b/java/rocksjni/backupablejni.cc → java/rocksjni/backup_engine_options.cc
diff --git a/java/rocksjni/portal.h b/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
    *

diff --git a/java/src/main/java/org/rocksdb/BackupEngine.java b/java/src/main/java/org/rocksdb/BackupEngine.java
@@ -226,8 +226,8 @@ public void restoreDbFromLatestBackup(
         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;

diff --git a/java/src/main/java/org/rocksdb/BackupEngineOptions.java b/java/src/main/java/org/rocksdb/BackupEngineOptions.java
@@ -8,8 +8,8 @@
 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>

diff --git a/java/src/test/java/org/rocksdb/BackupEngineOptionsTest.java b/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 @@ protected void log(InfoLogLevel infoLogLevel, String logMsg) {
 
             }
           }) {
-        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 void failShareFilesWithChecksumIfDisposed() {
   }
 
   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;
   }
 }