Fix some secondary/read-only DB logic (#13441)

Summary:
Primarily, fix an issue from https://github.com/facebook/rocksdb/issues/13316 with opening secondary DB with preserve/preclude option (crash test disable in https://github.com/facebook/rocksdb/issues/13439). The issue comes down to mixed-up interpretations of "read_only" which should now be resolved. I've introduced the stronger notion of "unchanging" which means the VersionSet never sees any changes to the LSM tree, and the weaker notion of "read_only" which means LSM tree changes are not written through this VersionSet/etc. but can pick up externally written changes. In particular, ManifestTailer should use read_only=true (along with unchanging=false) for proper handling of preserve/preclude options.

A new assertion in VersionSet::CreateColumnFamily to help ensure sane usage of the two boolean flags is incompatible with the known wart of allowing CreateColumnFamily on a read-only DB. So to keep that assertion, I have fixed that issue by disallowing it. And this in turn required downstream clean-up in ldb, where I cleaned up some call sites as well.

Also, rename SanitizeOptions for ColumnFamilyOptions to SanitizeCfOptions, for ease of search etc.

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

Test Plan:
* Added preserve option to a test in db_secondary_test, which reproduced the failure seen in the crash test.
* Revert https://github.com/facebook/rocksdb/issues/13439 to re-enable crash test functionality
* Update some tests to deal with disallowing CF creation on read-only DB
* Add some testing around read-only DBs and CreateColumnFamily(ies)
* Resurrect a nearby test for read-only DB to be sure it doesn't write to the DB dir. New EnforcedReadOnlyReopen should probably be used in more places but didn't want to attempt a big migration here and now. (Suggested follow-up.)

Reviewed By: jowlyzhang

Differential Revision: D70808033

Pulled By: pdillinger

fbshipit-source-id: 486b4e9f9c9045150a0ebb9cb302753d03932a3f
This commit is contained in:
Peter Dillinger 2025-03-07 14:56:45 -08:00 committed by Facebook GitHub Bot
parent 5d1c0a8832
commit b9c7481fc2
19 changed files with 161 additions and 84 deletions

View File

@ -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<size_t, 0xffffffff>,
@ -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_(

View File

@ -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.

View File

@ -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);

View File

@ -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<ColumnFamilyHandle*> 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<ColumnFamilyDescriptor> 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());

View File

@ -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);
}

View File

@ -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<std::string>& /*column_family_names*/,
std::vector<ColumnFamilyHandle*>* /*handles*/) override {
return Status::NotSupported("Not supported operation in read only mode.");
}
Status CreateColumnFamilies(
const std::vector<ColumnFamilyDescriptor>& /*column_families*/,
std::vector<ColumnFamilyHandle*>* /*handles*/) override {
return Status::NotSupported("Not supported operation in read only mode.");
}
// FIXME: some missing overrides for more "write" functions
protected:

View File

@ -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;

View File

@ -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"));

View File

@ -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<ReadOnlyFileSystem>(env_->GetFileSystem());
env_read_only_ = std::make_shared<CompositeEnvWrapper>(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();

View File

@ -1062,6 +1062,7 @@ class DBTestBase : public testing::Test {
MockEnv* mem_env_;
Env* encrypted_env_;
SpecialEnv* env_;
std::shared_ptr<Env> env_read_only_;
std::shared_ptr<Env> env_guard_;
DB* db_;
std::vector<ColumnFamilyHandle*> 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();

View File

@ -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

View File

@ -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,

View File

@ -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<ColumnFamilyDescriptor> 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,

View File

@ -5114,7 +5114,7 @@ VersionSet::VersionSet(
BlockCacheTracer* const block_cache_tracer,
const std::shared_ptr<IOTracer>& 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;

View File

@ -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<IOTracer>& 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<ColumnFamilyDescriptor>& 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<bool&>(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_;
};

View File

@ -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<bool> override_unchanging(&versions_->TEST_unchanging(),
read_only);
s = versions_->TryRecoverFromOneManifest(manifest_path, column_families,
read_only, &db_id,
&has_missing_table_file);

View File

@ -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

View File

@ -1421,7 +1421,7 @@ CompactorCommand::CompactorCommand(
const std::vector<std::string>& /*params*/,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& /*params*/,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& params,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& /*params*/,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& params,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& params,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& /*params*/,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& /*params*/,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<ColumnFamilyDescriptor> dummy;
ColumnFamilyDescriptor dummy_descriptor(kDefaultColumnFamilyName,
ColumnFamilyOptions(opt));
@ -2678,7 +2680,7 @@ ChangeCompactionStyleCommand::ChangeCompactionStyleCommand(
const std::vector<std::string>& /*params*/,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& /*params*/,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& params,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& params,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& params,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& /*params*/,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& params,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& /*params*/,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& params,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& params,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& params,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& params,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& params,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& /*params*/,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& /*params*/,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& /*params*/,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string>& /*params*/,
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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<std::string, std::string>& options,
const std::vector<std::string>& 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 {

View File

@ -0,0 +1 @@
* CreateColumnFamily() is no longer allowed on a read-only DB (OpenForReadOnly())