From 6534c6dea4e385e38f48fc561e717352d8a2d65d Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Tue, 5 Apr 2022 09:52:33 -0700 Subject: [PATCH] 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 | 4 +- CMakeLists.txt | 4 +- Makefile | 2 +- ROCKSDB_LITE.md | 2 +- TARGETS | 8 +- build_tools/run_ci_db_test.ps1 | 4 +- db/c_test.c | 2 +- db_stress_tool/db_stress_test_base.cc | 2 +- java/CMakeLists.txt | 2 +- ...kupablejni.cc => backup_engine_options.cc} | 0 java/rocksjni/portal.h | 2 +- .../main/java/org/rocksdb/BackupEngine.java | 4 +- .../java/org/rocksdb/BackupEngineOptions.java | 4 +- .../org/rocksdb/BackupEngineOptionsTest.java | 123 ++++++++---------- src.mk | 6 +- tools/ldb_cmd.cc | 23 ++-- tools/ldb_cmd_impl.h | 12 +- .../backup_engine.cc} | 14 +- .../backup_engine_impl.h} | 0 .../backup_engine_test.cc} | 41 +----- 20 files changed, 105 insertions(+), 154 deletions(-) rename java/rocksjni/{backupablejni.cc => backup_engine_options.cc} (100%) rename utilities/{backupable/backupable_db.cc => backup/backup_engine.cc} (99%) rename utilities/{backupable/backupable_db_impl.h => backup/backup_engine_impl.h} (100%) rename utilities/{backupable/backupable_db_test.cc => backup/backup_engine_test.cc} (99%) diff --git a/.travis.yml b/.travis.yml index 8c5ce5bb9..db249da73 100644 --- 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 index 01fd99253..6bb56fdb9 100644 --- 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 index c216c5458..f161d7d37 100644 --- 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 index 8991b9506..166426c60 100644 --- 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 index 067ea503a..de52c57fb 100644 --- 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 index 9aea51708..f20d3213f 100644 --- 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 index 980b5dd99..2a0bc8d5e 100644 --- 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 index 1368298eb..e6afb7cfc 100644 --- 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 index 872d27911..8eada17e8 100644 --- 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 b/java/rocksjni/backup_engine_options.cc similarity index 100% rename from java/rocksjni/backupablejni.cc rename to java/rocksjni/backup_engine_options.cc diff --git a/java/rocksjni/portal.h b/java/rocksjni/portal.h index 204370248..694db7b15 100644 --- 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 index 3f42b6334..515824a91 100644 --- a/java/src/main/java/org/rocksdb/BackupEngine.java +++ b/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; diff --git a/java/src/main/java/org/rocksdb/BackupEngineOptions.java b/java/src/main/java/org/rocksdb/BackupEngineOptions.java index a5501eda5..6e2dacc02 100644 --- a/java/src/main/java/org/rocksdb/BackupEngineOptions.java +++ b/java/src/main/java/org/rocksdb/BackupEngineOptions.java @@ -8,8 +8,8 @@ package org.rocksdb; import java.io.File; /** - *

BackupEngineOptions to control the behavior of a backupable database. - * It will be used during the creation of a {@link org.rocksdb.BackupEngine}. + *

BackupEngineOptions controls the behavior of a + * {@link org.rocksdb.BackupEngine}. *

*

Note that dispose() must be called before an Options instance * become out-of-scope to release the allocated memory in c++.

diff --git a/java/src/test/java/org/rocksdb/BackupEngineOptionsTest.java b/java/src/test/java/org/rocksdb/BackupEngineOptionsTest.java index f6ef36e7a..794bf04fb 100644 --- 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 @@ 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; } } diff --git a/src.mk b/src.mk index 705c6980c..faf82583f 100644 --- a/src.mk +++ b/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 \ diff --git a/tools/ldb_cmd.cc b/tools/ldb_cmd.cc index 0306260d4..66799df88 100644 --- a/tools/ldb_cmd.cc +++ b/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"; +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"; -BackupableCommand::BackupableCommand( +BackupEngineCommand::BackupEngineCommand( const std::vector& /*params*/, const std::map& options, const std::vector& 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& params, const std::map& options, const std::vector& 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& params, const std::map& options, const std::vector& 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() { diff --git a/tools/ldb_cmd_impl.h b/tools/ldb_cmd_impl.h index 4e0ab86ad..fbb6eb467 100644 --- a/tools/ldb_cmd_impl.h +++ b/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& params, - const std::map& options, - const std::vector& flags); + BackupEngineCommand(const std::vector& params, + const std::map& options, + const std::vector& 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& 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& params, diff --git a/utilities/backupable/backupable_db.cc b/utilities/backup/backup_engine.cc similarity index 99% rename from utilities/backupable/backupable_db.cc rename to utilities/backup/backup_engine.cc index d0b007e50..af633bff4 100644 --- a/utilities/backupable/backupable_db.cc +++ b/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 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 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, diff --git a/utilities/backupable/backupable_db_impl.h b/utilities/backup/backup_engine_impl.h similarity index 100% rename from utilities/backupable/backupable_db_impl.h rename to utilities/backup/backup_engine_impl.h diff --git a/utilities/backupable/backupable_db_test.cc b/utilities/backup/backup_engine_test.cc similarity index 99% rename from utilities/backupable/backupable_db_test.cc rename to utilities/backup/backup_engine_test.cc index ebcc2d69e..8585dbf12 100644 --- a/utilities/backupable/backupable_db_test.cc +++ b/utilities/backup/backup_engine_test.cc @@ -9,6 +9,8 @@ #if !defined(ROCKSDB_LITE) && !defined(OS_WIN) +#include "rocksdb/utilities/backup_engine.h" + #include #include #include @@ -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* 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: