diff --git a/db/column_family.cc b/db/column_family.cc index 8c6bf9c96b..ffb89c7540 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -200,9 +200,9 @@ const uint64_t kDefaultTtl = 0xfffffffffffffffe; const uint64_t kDefaultPeriodicCompSecs = 0xfffffffffffffffe; } // anonymous namespace -ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options, - bool read_only, - const ColumnFamilyOptions& src) { +ColumnFamilyOptions SanitizeCfOptions(const ImmutableDBOptions& db_options, + bool read_only, + const ColumnFamilyOptions& src) { ColumnFamilyOptions result = src; size_t clamp_max = std::conditional< sizeof(size_t) == 4, std::integral_constant, @@ -569,7 +569,7 @@ ColumnFamilyData::ColumnFamilyData( dropped_(false), flush_skip_reschedule_(false), internal_comparator_(cf_options.comparator), - initial_cf_options_(SanitizeOptions(db_options, read_only, cf_options)), + initial_cf_options_(SanitizeCfOptions(db_options, read_only, cf_options)), ioptions_(db_options, initial_cf_options_), mutable_cf_options_(initial_cf_options_), is_delete_range_supported_( diff --git a/db/column_family.h b/db/column_family.h index 5e18c90a1b..71401834ba 100644 --- a/db/column_family.h +++ b/db/column_family.h @@ -281,9 +281,9 @@ Status CheckConcurrentWritesSupported(const ColumnFamilyOptions& cf_options); Status CheckCFPathsSupported(const DBOptions& db_options, const ColumnFamilyOptions& cf_options); -ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options, - bool read_only, - const ColumnFamilyOptions& src); +ColumnFamilyOptions SanitizeCfOptions(const ImmutableDBOptions& db_options, + bool read_only, + const ColumnFamilyOptions& src); // Wrap user defined table properties collector factories `from cf_options` // into internal ones in internal_tbl_prop_coll_factories. Add a system internal // one too. diff --git a/db/column_family_test.cc b/db/column_family_test.cc index 224257df49..d84799b57c 100644 --- a/db/column_family_test.cc +++ b/db/column_family_test.cc @@ -271,8 +271,8 @@ class ColumnFamilyTestBase : public testing::Test { // them. ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions( ConfigOptions(), desc.options, - SanitizeOptions(dbfull()->immutable_db_options(), /*read_only*/ false, - current_cf_opt))); + SanitizeCfOptions(dbfull()->immutable_db_options(), + /*read_only*/ false, current_cf_opt))); cfi++; } } @@ -2233,7 +2233,7 @@ TEST_P(ColumnFamilyTest, CreateMissingColumnFamilies) { ASSERT_EQ(my_fs->options_files_created.load(), 2); } -TEST_P(ColumnFamilyTest, SanitizeOptions) { +TEST_P(ColumnFamilyTest, SanitizeCfOptions) { DBOptions db_options; for (int s = kCompactionStyleLevel; s <= kCompactionStyleUniversal; ++s) { for (int l = 0; l <= 2; l++) { @@ -2249,7 +2249,7 @@ TEST_P(ColumnFamilyTest, SanitizeOptions) { original.write_buffer_size = l * 4 * 1024 * 1024 + i * 1024 * 1024 + j * 1024 + k; - ColumnFamilyOptions result = SanitizeOptions( + ColumnFamilyOptions result = SanitizeCfOptions( ImmutableDBOptions(db_options), /*read_only*/ false, original); ASSERT_TRUE(result.level0_stop_writes_trigger >= result.level0_slowdown_writes_trigger); diff --git a/db/db_basic_test.cc b/db/db_basic_test.cc index 5ab97a7340..08b6486df9 100644 --- a/db/db_basic_test.cc +++ b/db/db_basic_test.cc @@ -161,6 +161,7 @@ TEST_F(DBBasicTest, UniqueSession) { ASSERT_EQ(sid2, sid3); + DestroyAndReopen(options); CreateAndReopenWithCF({"goku"}, options); ASSERT_OK(db_->GetDbSessionId(sid1)); ASSERT_OK(Put("bar", "e1")); @@ -179,6 +180,7 @@ TEST_F(DBBasicTest, UniqueSession) { TEST_F(DBBasicTest, ReadOnlyDB) { ASSERT_OK(Put("foo", "v1")); ASSERT_OK(Put("bar", "v2")); + ASSERT_OK(Flush()); ASSERT_OK(Put("foo", "v3")); Close(); @@ -208,10 +210,11 @@ TEST_F(DBBasicTest, ReadOnlyDB) { auto options = CurrentOptions(); assert(options.env == env_); - ASSERT_OK(ReadOnlyReopen(options)); + ASSERT_OK(EnforcedReadOnlyReopen(options)); ASSERT_EQ("v3", Get("foo")); ASSERT_EQ("v2", Get("bar")); verify_all_iters(); + ASSERT_EQ(Flush().code(), Status::Code::kNotSupported); Close(); // Reopen and flush memtable. @@ -219,26 +222,38 @@ TEST_F(DBBasicTest, ReadOnlyDB) { ASSERT_OK(Flush()); Close(); // Now check keys in read only mode. - ASSERT_OK(ReadOnlyReopen(options)); + ASSERT_OK(EnforcedReadOnlyReopen(options)); ASSERT_EQ("v3", Get("foo")); ASSERT_EQ("v2", Get("bar")); verify_all_iters(); - ASSERT_TRUE(db_->SyncWAL().IsNotSupported()); + ASSERT_EQ(db_->SyncWAL().code(), Status::Code::kNotSupported); + + // More ops that should fail + std::vector cfhs{{}}; + ASSERT_EQ(db_->CreateColumnFamily(options, "blah", &cfhs[0]).code(), + Status::Code::kNotSupported); + + ASSERT_EQ(db_->CreateColumnFamilies(options, {"blah"}, &cfhs).code(), + Status::Code::kNotSupported); + + std::vector cfds; + cfds.push_back({"blah", options}); + ASSERT_EQ(db_->CreateColumnFamilies(cfds, &cfhs).code(), + Status::Code::kNotSupported); } -// TODO akanksha: Update the test to check that combination -// does not actually write to FS (use open read-only with -// CompositeEnvWrapper+ReadOnlyFileSystem). -TEST_F(DBBasicTest, DISABLED_ReadOnlyDBWithWriteDBIdToManifestSet) { +TEST_F(DBBasicTest, ReadOnlyDBWithWriteDBIdToManifestSet) { + auto options = CurrentOptions(); + options.write_dbid_to_manifest = false; + DestroyAndReopen(options); ASSERT_OK(Put("foo", "v1")); ASSERT_OK(Put("bar", "v2")); ASSERT_OK(Put("foo", "v3")); Close(); - auto options = CurrentOptions(); options.write_dbid_to_manifest = true; assert(options.env == env_); - ASSERT_OK(ReadOnlyReopen(options)); + ASSERT_OK(EnforcedReadOnlyReopen(options)); std::string db_id1; ASSERT_OK(db_->GetDbIdentity(db_id1)); ASSERT_EQ("v3", Get("foo")); @@ -258,7 +273,7 @@ TEST_F(DBBasicTest, DISABLED_ReadOnlyDBWithWriteDBIdToManifestSet) { ASSERT_OK(Flush()); Close(); // Now check keys in read only mode. - ASSERT_OK(ReadOnlyReopen(options)); + ASSERT_OK(EnforcedReadOnlyReopen(options)); ASSERT_EQ("v3", Get("foo")); ASSERT_EQ("v2", Get("bar")); ASSERT_TRUE(db_->SyncWAL().IsNotSupported()); diff --git a/db/db_impl/db_impl_open.cc b/db/db_impl/db_impl_open.cc index 22b1cfd7c7..549e574f7f 100644 --- a/db/db_impl/db_impl_open.cc +++ b/db/db_impl/db_impl_open.cc @@ -35,8 +35,8 @@ Options SanitizeOptions(const std::string& dbname, const Options& src, auto db_options = SanitizeOptions(dbname, DBOptions(src), read_only, logger_creation_s); ImmutableDBOptions immutable_db_options(db_options); - auto cf_options = SanitizeOptions(immutable_db_options, read_only, - ColumnFamilyOptions(src)); + auto cf_options = SanitizeCfOptions(immutable_db_options, read_only, + ColumnFamilyOptions(src)); return Options(db_options, cf_options); } diff --git a/db/db_impl/db_impl_readonly.h b/db/db_impl/db_impl_readonly.h index 9566f547bf..3edfeb0e55 100644 --- a/db/db_impl/db_impl_readonly.h +++ b/db/db_impl/db_impl_readonly.h @@ -155,6 +155,29 @@ class DBImplReadOnly : public DBImpl { return Status::NotSupported("Not supported operation in read only mode."); } + using DB::CreateColumnFamily; + using DBImpl::CreateColumnFamily; + Status CreateColumnFamily(const ColumnFamilyOptions& /*cf_options*/, + const std::string& /*column_family*/, + ColumnFamilyHandle** /*handle*/) override { + return Status::NotSupported("Not supported operation in read only mode."); + } + + using DB::CreateColumnFamilies; + using DBImpl::CreateColumnFamilies; + Status CreateColumnFamilies( + const ColumnFamilyOptions& /*cf_options*/, + const std::vector& /*column_family_names*/, + std::vector* /*handles*/) override { + return Status::NotSupported("Not supported operation in read only mode."); + } + + Status CreateColumnFamilies( + const std::vector& /*column_families*/, + std::vector* /*handles*/) override { + return Status::NotSupported("Not supported operation in read only mode."); + } + // FIXME: some missing overrides for more "write" functions protected: diff --git a/db/db_options_test.cc b/db/db_options_test.cc index d619e8604e..df0d4ca3c7 100644 --- a/db/db_options_test.cc +++ b/db/db_options_test.cc @@ -71,7 +71,7 @@ class DBOptionsTest : public DBTestBase { ImmutableDBOptions db_options(options); test::RandomInitCFOptions(&options, options, rnd); auto sanitized_options = - SanitizeOptions(db_options, /*read_only*/ false, options); + SanitizeCfOptions(db_options, /*read_only*/ false, options); auto opt_map = GetMutableCFOptionsMap(sanitized_options); delete options.compaction_filter; return opt_map; diff --git a/db/db_secondary_test.cc b/db/db_secondary_test.cc index 060ce86440..5be4feecf7 100644 --- a/db/db_secondary_test.cc +++ b/db/db_secondary_test.cc @@ -160,6 +160,7 @@ TEST_F(DBSecondaryTest, NonExistingDb) { TEST_F(DBSecondaryTest, ReopenAsSecondary) { Options options; options.env = env_; + options.preserve_internal_time_seconds = 300; Reopen(options); ASSERT_OK(Put("foo", "foo_value")); ASSERT_OK(Put("bar", "bar_value")); diff --git a/db/db_test_util.cc b/db/db_test_util.cc index 3944e92a0d..64a85bc410 100644 --- a/db/db_test_util.cc +++ b/db/db_test_util.cc @@ -11,6 +11,7 @@ #include "cache/cache_reservation_manager.h" #include "db/forward_iterator.h" +#include "env/fs_readonly.h" #include "env/mock_env.h" #include "port/lang.h" #include "rocksdb/cache.h" @@ -716,6 +717,17 @@ Status DBTestBase::ReadOnlyReopen(const Options& options) { return DB::OpenForReadOnly(options, dbname_, &db_); } +Status DBTestBase::EnforcedReadOnlyReopen(const Options& options) { + Close(); + Options options_copy = options; + MaybeInstallTimeElapseOnlySleep(options_copy); + auto fs_read_only = + std::make_shared(env_->GetFileSystem()); + env_read_only_ = std::make_shared(env_, fs_read_only); + options_copy.env = env_read_only_.get(); + return DB::OpenForReadOnly(options_copy, dbname_, &db_); +} + Status DBTestBase::TryReopen(const Options& options) { Close(); last_options_.table_factory.reset(); diff --git a/db/db_test_util.h b/db/db_test_util.h index 1ddb4faef1..4a00ea4371 100644 --- a/db/db_test_util.h +++ b/db/db_test_util.h @@ -1062,6 +1062,7 @@ class DBTestBase : public testing::Test { MockEnv* mem_env_; Env* encrypted_env_; SpecialEnv* env_; + std::shared_ptr env_read_only_; std::shared_ptr env_guard_; DB* db_; std::vector handles_; @@ -1178,6 +1179,9 @@ class DBTestBase : public testing::Test { Status ReadOnlyReopen(const Options& options); + // With a filesystem wrapper that fails on attempted write + Status EnforcedReadOnlyReopen(const Options& options); + Status TryReopen(const Options& options); bool IsDirectIOSupported(); diff --git a/db/repair.cc b/db/repair.cc index 3918940293..0c108a6016 100644 --- a/db/repair.cc +++ b/db/repair.cc @@ -100,13 +100,15 @@ class Repairer { db_options_(SanitizeOptions(dbname_, db_options)), immutable_db_options_(ImmutableDBOptions(db_options_)), icmp_(default_cf_opts.comparator), - default_cf_opts_(SanitizeOptions(immutable_db_options_, - /*read_only*/ false, default_cf_opts)), + default_cf_opts_(SanitizeCfOptions(immutable_db_options_, + /*read_only*/ false, + default_cf_opts)), default_iopts_( ImmutableOptions(immutable_db_options_, default_cf_opts_)), default_mopts_(MutableCFOptions(default_cf_opts_)), - unknown_cf_opts_(SanitizeOptions(immutable_db_options_, - /*read_only*/ false, unknown_cf_opts)), + unknown_cf_opts_(SanitizeCfOptions(immutable_db_options_, + /*read_only*/ false, + unknown_cf_opts)), create_unknown_cfs_(create_unknown_cfs), raw_table_cache_( // TableCache can be small since we expect each table to be opened diff --git a/db/version_edit_handler.cc b/db/version_edit_handler.cc index d1b5ee68ce..e60644e271 100644 --- a/db/version_edit_handler.cc +++ b/db/version_edit_handler.cc @@ -408,7 +408,7 @@ void VersionEditHandler::CheckIterationResult(const log::Reader& reader, if (cfd->IsDropped()) { continue; } - if (read_only_) { + if (version_set_->unchanging()) { cfd->table_cache()->SetTablesAreImmortal(); } *s = LoadTables(cfd, /*prefetch_index_and_filter_in_cache=*/false, diff --git a/db/version_edit_handler.h b/db/version_edit_handler.h index f3637ae730..0cef558826 100644 --- a/db/version_edit_handler.h +++ b/db/version_edit_handler.h @@ -198,7 +198,9 @@ class VersionEditHandler : public VersionEditHandlerBase { bool prefetch_index_and_filter_in_cache, bool is_initial_load); - virtual bool MustOpenAllColumnFamilies() const { return !read_only_; } + virtual bool MustOpenAllColumnFamilies() const { + return !version_set_->unchanging(); + } const bool read_only_; std::vector column_families_; @@ -334,10 +336,10 @@ class ManifestTailer : public VersionEditHandlerPointInTime { const ReadOptions& read_options, EpochNumberRequirement epoch_number_requirement = EpochNumberRequirement::kMustPresent) - : VersionEditHandlerPointInTime(/*read_only=*/false, column_families, - version_set, io_tracer, read_options, - /*allow_incomplete_valid_version=*/false, - epoch_number_requirement), + : VersionEditHandlerPointInTime( + /*read_only=*/true, column_families, version_set, io_tracer, + read_options, + /*allow_incomplete_valid_version=*/false, epoch_number_requirement), mode_(Mode::kRecovery) {} Status VerifyFile(ColumnFamilyData* cfd, const std::string& fpath, int level, diff --git a/db/version_set.cc b/db/version_set.cc index da1ad3ea87..7e9893a93c 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -5114,7 +5114,7 @@ VersionSet::VersionSet( BlockCacheTracer* const block_cache_tracer, const std::shared_ptr& io_tracer, const std::string& db_id, const std::string& db_session_id, const std::string& daily_offpeak_time_utc, - ErrorHandler* const error_handler, const bool read_only) + ErrorHandler* error_handler, bool unchanging) : column_family_set_(new ColumnFamilySet( dbname, _db_options, storage_options, table_cache, write_buffer_manager, write_controller, block_cache_tracer, io_tracer, @@ -5143,12 +5143,12 @@ VersionSet::VersionSet( db_session_id_(db_session_id), offpeak_time_option_(OffpeakTimeOption(daily_offpeak_time_utc)), error_handler_(error_handler), - read_only_(read_only), + unchanging_(unchanging), closed_(false) {} Status VersionSet::Close(FSDirectory* db_dir, InstrumentedMutex* mu) { Status s; - if (closed_ || read_only_ || !manifest_file_number_ || !descriptor_log_) { + if (closed_ || unchanging_ || !manifest_file_number_ || !descriptor_log_) { return s; } @@ -7297,6 +7297,8 @@ ColumnFamilyData* VersionSet::CreateColumnFamily( const ColumnFamilyOptions& cf_options, const ReadOptions& read_options, const VersionEdit* edit, bool read_only) { assert(edit->IsColumnFamilyAdd()); + // Unchanging LSM tree implies no writes to the CF + assert(!unchanging_ || read_only); MutableCFOptions dummy_cf_options; Version* dummy_versions = @@ -7430,7 +7432,7 @@ ReactiveVersionSet::ReactiveVersionSet( write_buffer_manager, write_controller, /*block_cache_tracer=*/nullptr, io_tracer, /*db_id*/ "", /*db_session_id*/ "", /*daily_offpeak_time_utc*/ "", - /*error_handler=*/nullptr, /*read_only=*/true) {} + /*error_handler=*/nullptr, /*unchanging=*/false) {} ReactiveVersionSet::~ReactiveVersionSet() = default; diff --git a/db/version_set.h b/db/version_set.h index d9cc5a8e07..6d6ee5c486 100644 --- a/db/version_set.h +++ b/db/version_set.h @@ -1174,6 +1174,9 @@ class AtomicGroupReadBuffer { // VersionSet is the collection of versions of all the column families of the // database. Each database owns one VersionSet. A VersionSet has access to all // column families via ColumnFamilySet, i.e. set of the column families. +// `unchanging` means the LSM tree structure of the column families will not +// change during the lifetime of this VersionSet (true for read-only instance, +// but false for secondary instance or writable DB). class VersionSet { public: VersionSet(const std::string& dbname, const ImmutableDBOptions* db_options, @@ -1184,7 +1187,7 @@ class VersionSet { const std::shared_ptr& io_tracer, const std::string& db_id, const std::string& db_session_id, const std::string& daily_offpeak_time_utc, - ErrorHandler* const error_handler, const bool read_only); + ErrorHandler* error_handler, bool unchanging); // No copying allowed VersionSet(const VersionSet&) = delete; void operator=(const VersionSet&) = delete; @@ -1263,8 +1266,11 @@ class VersionSet { void WakeUpWaitingManifestWriters(); // Recover the last saved descriptor (MANIFEST) from persistent storage. - // If read_only == true, Recover() will not complain if some column families - // are not opened + // Unlike `unchanging` on the VersionSet, `read_only` here and in other + // functions below refers to the CF receiving no writes or modifications + // through this VersionSet, but could through external manifest updates + // etc. Thus, `read_only=true` for secondary instances as well as read-only + // instances. Status Recover(const std::vector& column_families, bool read_only = false, std::string* db_id = nullptr, bool no_error_if_files_missing = false, bool is_retry = false, @@ -1342,6 +1348,8 @@ class VersionSet { return min_log_number_to_keep_.load(); } + bool unchanging() const { return unchanging_; } + // Allocate and return a new file number uint64_t NewFileNumber() { return next_file_number_.fetch_add(1); } @@ -1573,6 +1581,8 @@ class VersionSet { AppendVersion(cfd, version); } + bool& TEST_unchanging() { return const_cast(unchanging_); } + protected: struct ManifestWriter; @@ -1722,7 +1732,7 @@ class VersionSet { VersionEdit* edit, SequenceNumber* max_last_sequence, InstrumentedMutex* mu); - const bool read_only_; + const bool unchanging_; bool closed_; }; diff --git a/db/version_set_test.cc b/db/version_set_test.cc index c249fa6daf..65cee38de1 100644 --- a/db/version_set_test.cc +++ b/db/version_set_test.cc @@ -26,6 +26,7 @@ #include "test_util/mock_time_env.h" #include "test_util/testharness.h" #include "test_util/testutil.h" +#include "util/defer.h" #include "util/string_util.h" namespace ROCKSDB_NAMESPACE { @@ -1905,7 +1906,7 @@ TEST_F(VersionSetTest, WalAddition) { &write_buffer_manager_, &write_controller_, /*block_cache_tracer=*/nullptr, /*io_tracer=*/nullptr, /*db_id=*/"", /*db_session_id=*/"", /*daily_offpeak_time_utc=*/"", - /*error_handler=*/nullptr, /*read_only=*/false)); + /*error_handler=*/nullptr, /*unchanging=*/false)); ASSERT_OK(new_versions->Recover(column_families_, /*read_only=*/false)); const auto& wals = new_versions->GetWalSet().GetWals(); ASSERT_EQ(wals.size(), 1); @@ -1973,7 +1974,7 @@ TEST_F(VersionSetTest, WalCloseWithoutSync) { &write_buffer_manager_, &write_controller_, /*block_cache_tracer=*/nullptr, /*io_tracer=*/nullptr, /*db_id=*/"", /*db_session_id=*/"", /*daily_offpeak_time_utc=*/"", - /*error_handler=*/nullptr, /*read_only=*/false)); + /*error_handler=*/nullptr, /*unchanging=*/false)); ASSERT_OK(new_versions->Recover(column_families_, false)); const auto& wals = new_versions->GetWalSet().GetWals(); ASSERT_EQ(wals.size(), 2); @@ -2027,7 +2028,7 @@ TEST_F(VersionSetTest, WalDeletion) { &write_buffer_manager_, &write_controller_, /*block_cache_tracer=*/nullptr, /*io_tracer=*/nullptr, /*db_id=*/"", /*db_session_id=*/"", /*daily_offpeak_time_utc=*/"", - /*error_handler=*/nullptr, /*read_only=*/false)); + /*error_handler=*/nullptr, /*unchanging=*/false)); ASSERT_OK(new_versions->Recover(column_families_, false)); const auto& wals = new_versions->GetWalSet().GetWals(); ASSERT_EQ(wals.size(), 1); @@ -2066,7 +2067,7 @@ TEST_F(VersionSetTest, WalDeletion) { &write_buffer_manager_, &write_controller_, /*block_cache_tracer=*/nullptr, /*io_tracer=*/nullptr, /*db_id=*/"", /*db_session_id=*/"", /*daily_offpeak_time_utc=*/"", - /*error_handler=*/nullptr, /*read_only=*/false)); + /*error_handler=*/nullptr, /*unchanging=*/false)); ASSERT_OK(new_versions->Recover(column_families_, false)); const auto& wals = new_versions->GetWalSet().GetWals(); ASSERT_EQ(wals.size(), 1); @@ -2187,7 +2188,7 @@ TEST_F(VersionSetTest, DeleteWalsBeforeNonExistingWalNumber) { &write_buffer_manager_, &write_controller_, /*block_cache_tracer=*/nullptr, /*io_tracer=*/nullptr, /*db_id=*/"", /*db_session_id=*/"", /*daily_offpeak_time_utc=*/"", - /*error_handler=*/nullptr, /*read_only=*/false)); + /*error_handler=*/nullptr, /*unchanging=*/false)); ASSERT_OK(new_versions->Recover(column_families_, false)); const auto& wals = new_versions->GetWalSet().GetWals(); ASSERT_EQ(wals.size(), 1); @@ -2224,7 +2225,7 @@ TEST_F(VersionSetTest, DeleteAllWals) { &write_buffer_manager_, &write_controller_, /*block_cache_tracer=*/nullptr, /*io_tracer=*/nullptr, /*db_id=*/"", /*db_session_id=*/"", /*daily_offpeak_time_utc=*/"", - /*error_handler=*/nullptr, /*read_only=*/false)); + /*error_handler=*/nullptr, /*unchanging=*/false)); ASSERT_OK(new_versions->Recover(column_families_, false)); const auto& wals = new_versions->GetWalSet().GetWals(); ASSERT_EQ(wals.size(), 0); @@ -2267,7 +2268,7 @@ TEST_F(VersionSetTest, AtomicGroupWithWalEdits) { &write_buffer_manager_, &write_controller_, /*block_cache_tracer=*/nullptr, /*io_tracer=*/nullptr, /*db_id=*/"", /*db_session_id=*/"", /*daily_offpeak_time_utc=*/"", - /*error_handler=*/nullptr, /*read_only=*/false)); + /*error_handler=*/nullptr, /*unchanging=*/false)); std::string db_id; ASSERT_OK( new_versions->Recover(column_families_, /*read_only=*/false, &db_id)); @@ -2447,7 +2448,7 @@ class VersionSetWithTimestampTest : public VersionSetTest { &write_buffer_manager_, &write_controller_, /*block_cache_tracer=*/nullptr, /*io_tracer=*/nullptr, /*db_id=*/"", /*db_session_id=*/"", /*daily_offpeak_time_utc=*/"", - /*error_handler=*/nullptr, /*read_only=*/false)); + /*error_handler=*/nullptr, /*unchanging=*/false)); ASSERT_OK(vset->Recover(column_families_, /*read_only=*/false, /*db_id=*/nullptr)); for (auto* cfd : *(vset->GetColumnFamilySet())) { @@ -3749,6 +3750,8 @@ TEST_P(VersionSetTestEmptyDb, OpenCompleteManifest) { } std::string db_id; bool has_missing_table_file = false; + SaveAndRestore override_unchanging(&versions_->TEST_unchanging(), + read_only); s = versions_->TryRecoverFromOneManifest(manifest_path, column_families, read_only, &db_id, &has_missing_table_file); diff --git a/tools/db_crashtest.py b/tools/db_crashtest.py index 6503260658..831de21fd9 100644 --- a/tools/db_crashtest.py +++ b/tools/db_crashtest.py @@ -1030,9 +1030,6 @@ def finalize_and_sanitize(src_params): # Continuous verification fails with secondaries inside NonBatchedOpsStressTest if dest_params.get("test_secondary") == 1: dest_params["continuous_verification_interval"] = 0 - # FIXME: temporarily broken combination - dest_params["preserve_internal_time_seconds"] = 0 - dest_params["preclude_last_level_data_seconds"] = 0 return dest_params diff --git a/tools/ldb_cmd.cc b/tools/ldb_cmd.cc index 16a47ab5b0..3b8a293373 100644 --- a/tools/ldb_cmd.cc +++ b/tools/ldb_cmd.cc @@ -1421,7 +1421,7 @@ CompactorCommand::CompactorCommand( const std::vector& /*params*/, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, false, + : LDBCommand(options, flags, false /* is_read_only */, BuildCmdLineOptions({ARG_FROM, ARG_TO, ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_TTL})), null_from_(true), @@ -1496,7 +1496,7 @@ DBLoaderCommand::DBLoaderCommand( const std::map& options, const std::vector& flags) : LDBCommand( - options, flags, false, + options, flags, false /* is_read_only */, BuildCmdLineOptions({ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_FROM, ARG_TO, ARG_CREATE_IF_MISSING, ARG_DISABLE_WAL, ARG_BULK_LOAD, ARG_COMPACT})), @@ -1628,7 +1628,7 @@ ManifestDumpCommand::ManifestDumpCommand( const std::map& options, const std::vector& flags) : LDBCommand( - options, flags, false, + options, flags, true /* is_read_only */, BuildCmdLineOptions({ARG_VERBOSE, ARG_PATH, ARG_HEX, ARG_JSON})), verbose_(false), json_(false) { @@ -1776,7 +1776,7 @@ FileChecksumDumpCommand::FileChecksumDumpCommand( const std::vector& /*params*/, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, false, + : LDBCommand(options, flags, true /* is_read_only */, BuildCmdLineOptions({ARG_PATH, ARG_HEX})) { auto itr = options.find(ARG_PATH); if (itr != options.end()) { @@ -1840,7 +1840,8 @@ GetPropertyCommand::GetPropertyCommand( const std::vector& params, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, true, BuildCmdLineOptions({})) { + : LDBCommand(options, flags, true /* is_read_only */, + BuildCmdLineOptions({})) { if (params.size() != 1) { exec_state_ = LDBCommandExecuteResult::Failed("property name must be specified"); @@ -1891,7 +1892,8 @@ ListColumnFamiliesCommand::ListColumnFamiliesCommand( const std::vector& /*params*/, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, false, BuildCmdLineOptions({})) {} + : LDBCommand(options, flags, true /* is_read_only */, + BuildCmdLineOptions({})) {} void ListColumnFamiliesCommand::DoCommand() { PrepareOptions(); @@ -1925,7 +1927,7 @@ CreateColumnFamilyCommand::CreateColumnFamilyCommand( const std::vector& params, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, true, {ARG_DB}) { + : LDBCommand(options, flags, false /* is_read_only */, {ARG_DB}) { if (params.size() != 1) { exec_state_ = LDBCommandExecuteResult::Failed( "new column family name must be specified"); @@ -1962,7 +1964,7 @@ DropColumnFamilyCommand::DropColumnFamilyCommand( const std::vector& params, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, true, {ARG_DB}) { + : LDBCommand(options, flags, false /* is_read_only */, {ARG_DB}) { if (params.size() != 1) { exec_state_ = LDBCommandExecuteResult::Failed( "The name of column family to drop must be specified"); @@ -2038,7 +2040,7 @@ InternalDumpCommand::InternalDumpCommand( const std::vector& /*params*/, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, true, + : LDBCommand(options, flags, true /* is_read_only */, BuildCmdLineOptions( {ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_FROM, ARG_TO, ARG_MAX_KEYS, ARG_COUNT_ONLY, ARG_COUNT_DELIM, ARG_STATS, @@ -2219,7 +2221,7 @@ DBDumperCommand::DBDumperCommand( const std::map& options, const std::vector& flags) : LDBCommand( - options, flags, true, + options, flags, true /* is_read_only */, BuildCmdLineOptions( {ARG_TTL, ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_FROM, ARG_TO, ARG_MAX_KEYS, ARG_COUNT_ONLY, ARG_COUNT_DELIM, ARG_STATS, @@ -2539,7 +2541,7 @@ ReduceDBLevelsCommand::ReduceDBLevelsCommand( const std::vector& /*params*/, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, false, + : LDBCommand(options, flags, false /* is_read_only */, BuildCmdLineOptions({ARG_NEW_LEVELS, ARG_PRINT_OLD_LEVELS})), old_levels_(1 << 7), new_levels_(-1), @@ -2596,7 +2598,7 @@ Status ReduceDBLevelsCommand::GetOldNumOfLevels(Options& opt, int* levels) { /*block_cache_tracer=*/nullptr, /*io_tracer=*/nullptr, /*db_id=*/"", /*db_session_id=*/"", opt.daily_offpeak_time_utc, - /*error_handler=*/nullptr, /*read_only=*/true); + /*error_handler=*/nullptr, /*unchanging=*/false); std::vector dummy; ColumnFamilyDescriptor dummy_descriptor(kDefaultColumnFamilyName, ColumnFamilyOptions(opt)); @@ -2678,7 +2680,7 @@ ChangeCompactionStyleCommand::ChangeCompactionStyleCommand( const std::vector& /*params*/, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, false, + : LDBCommand(options, flags, false /* is_read_only */, BuildCmdLineOptions( {ARG_OLD_COMPACTION_STYLE, ARG_NEW_COMPACTION_STYLE})), old_compaction_style_(-1), @@ -3224,7 +3226,7 @@ WALDumperCommand::WALDumperCommand( const std::vector& /*params*/, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, true, + : LDBCommand(options, flags, true /* is_read_only */, BuildCmdLineOptions({ARG_WAL_FILE, ARG_DB, ARG_WRITE_COMMITTED, ARG_PRINT_HEADER, ARG_PRINT_VALUE, ARG_ONLY_PRINT_SEQNO_GAPS})), @@ -3280,7 +3282,7 @@ void WALDumperCommand::DoCommand() { GetCommand::GetCommand(const std::vector& params, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, true, + : LDBCommand(options, flags, true /* is_read_only */, BuildCmdLineOptions({ARG_TTL, ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_READ_TIMESTAMP})) { if (params.size() != 1) { @@ -3339,7 +3341,7 @@ MultiGetCommand::MultiGetCommand( const std::vector& params, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, true, + : LDBCommand(options, flags, true /* is_read_only */, BuildCmdLineOptions({ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_READ_TIMESTAMP})) { if (params.size() < 1) { @@ -3414,7 +3416,7 @@ GetEntityCommand::GetEntityCommand( const std::vector& params, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, true, + : LDBCommand(options, flags, true /* is_read_only */, BuildCmdLineOptions({ARG_TTL, ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_READ_TIMESTAMP})) { if (params.size() != 1) { @@ -3552,7 +3554,7 @@ ApproxSizeCommand::ApproxSizeCommand( const std::vector& /*params*/, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, true, + : LDBCommand(options, flags, true /* is_read_only */, BuildCmdLineOptions( {ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_FROM, ARG_TO})) { if (options.find(ARG_FROM) != options.end()) { @@ -3608,7 +3610,7 @@ BatchPutCommand::BatchPutCommand( const std::vector& params, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, false, + : LDBCommand(options, flags, false /* is_read_only */, BuildCmdLineOptions({ARG_TTL, ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_CREATE_IF_MISSING})) { if (params.size() < 2) { @@ -3680,7 +3682,7 @@ ScanCommand::ScanCommand(const std::vector& /*params*/, const std::map& options, const std::vector& flags) : LDBCommand( - options, flags, true, + options, flags, true /* is_read_only */, BuildCmdLineOptions({ARG_TTL, ARG_NO_VALUE, ARG_HEX, ARG_KEY_HEX, ARG_TO, ARG_VALUE_HEX, ARG_FROM, ARG_TIMESTAMP, ARG_MAX_KEYS, ARG_TTL_START, ARG_TTL_END, @@ -3857,7 +3859,7 @@ void ScanCommand::DoCommand() { DeleteCommand::DeleteCommand(const std::vector& params, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, false, + : LDBCommand(options, flags, false /* is_read_only */, BuildCmdLineOptions({ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX})) { if (params.size() != 1) { exec_state_ = LDBCommandExecuteResult::Failed( @@ -3893,7 +3895,7 @@ SingleDeleteCommand::SingleDeleteCommand( const std::vector& params, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, false, + : LDBCommand(options, flags, false /* is_read_only */, BuildCmdLineOptions({ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX})) { if (params.size() != 1) { exec_state_ = LDBCommandExecuteResult::Failed( @@ -3929,7 +3931,7 @@ DeleteRangeCommand::DeleteRangeCommand( const std::vector& params, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, false, + : LDBCommand(options, flags, false /* is_read_only */, BuildCmdLineOptions({ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX})) { if (params.size() != 2) { exec_state_ = LDBCommandExecuteResult::Failed( @@ -3967,7 +3969,7 @@ void DeleteRangeCommand::DoCommand() { PutCommand::PutCommand(const std::vector& params, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, false, + : LDBCommand(options, flags, false /* is_read_only */, BuildCmdLineOptions({ARG_TTL, ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_CREATE_IF_MISSING})) { if (params.size() != 2) { @@ -4021,7 +4023,7 @@ PutEntityCommand::PutEntityCommand( const std::vector& params, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, false, + : LDBCommand(options, flags, false /* is_read_only */, BuildCmdLineOptions({ARG_TTL, ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_CREATE_IF_MISSING})) { if (params.size() < 2) { @@ -4103,7 +4105,7 @@ DBQuerierCommand::DBQuerierCommand( const std::map& options, const std::vector& flags) : LDBCommand( - options, flags, false, + options, flags, false /* is_read_only */, BuildCmdLineOptions({ARG_TTL, ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX})) { } @@ -4339,7 +4341,8 @@ CheckConsistencyCommand::CheckConsistencyCommand( const std::vector& /*params*/, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, true, BuildCmdLineOptions({})) {} + : LDBCommand(options, flags, true /* is_read_only */, + BuildCmdLineOptions({})) {} void CheckConsistencyCommand::Help(std::string& ret) { ret.append(" "); @@ -4402,7 +4405,8 @@ const std::string RepairCommand::ARG_VERBOSE = "verbose"; RepairCommand::RepairCommand(const std::vector& /*params*/, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, false, BuildCmdLineOptions({ARG_VERBOSE})) { + : LDBCommand(options, flags, false /* is_read_only */, + BuildCmdLineOptions({ARG_VERBOSE})) { verbose_ = IsFlagPresent(flags, ARG_VERBOSE); } @@ -4683,7 +4687,7 @@ DBFileDumperCommand::DBFileDumperCommand( const std::vector& /*params*/, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, true, + : LDBCommand(options, flags, true /* is_read_only */, BuildCmdLineOptions( {ARG_DECODE_BLOB_INDEX, ARG_DUMP_UNCOMPRESSED_BLOBS})), decode_blob_index_(IsFlagPresent(flags, ARG_DECODE_BLOB_INDEX)), @@ -4804,7 +4808,7 @@ DBLiveFilesMetadataDumperCommand::DBLiveFilesMetadataDumperCommand( const std::vector& /*params*/, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, true, + : LDBCommand(options, flags, true /* is_read_only */, BuildCmdLineOptions({ARG_SORT_BY_FILENAME})) { sort_by_filename_ = IsFlagPresent(flags, ARG_SORT_BY_FILENAME); } @@ -5119,7 +5123,8 @@ void IngestExternalSstFilesCommand::OverrideBaseOptions() { ListFileRangeDeletesCommand::ListFileRangeDeletesCommand( const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, true, BuildCmdLineOptions({ARG_MAX_KEYS})) { + : LDBCommand(options, flags, true /* is_read_only */, + BuildCmdLineOptions({ARG_MAX_KEYS})) { auto itr = options.find(ARG_MAX_KEYS); if (itr != options.end()) { try { diff --git a/unreleased_history/behavior_changes/read_only_create_cf.md b/unreleased_history/behavior_changes/read_only_create_cf.md new file mode 100644 index 0000000000..2ff8e658a7 --- /dev/null +++ b/unreleased_history/behavior_changes/read_only_create_cf.md @@ -0,0 +1 @@ +* CreateColumnFamily() is no longer allowed on a read-only DB (OpenForReadOnly())