diff --git a/db/column_family.cc b/db/column_family.cc index ce22a00aac3..531cbeca681 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -1148,13 +1148,60 @@ void ColumnFamilyData::ResetThreadLocalSuperVersions() { } } +Status ColumnFamilyData::ValidateOptions( + const DBOptions& db_options, const ColumnFamilyOptions& cf_options) { + Status s; + s = CheckCompressionSupported(cf_options); + if (s.ok() && db_options.allow_concurrent_memtable_write) { + s = CheckConcurrentWritesSupported(cf_options); + } + if (s.ok()) { + s = CheckCFPathsSupported(db_options, cf_options); + } + if (!s.ok()) { + return s; + } + + if (cf_options.ttl > 0) { + if (db_options.max_open_files != -1) { + return Status::NotSupported( + "TTL is only supported when files are always " + "kept open (set max_open_files = -1). "); + } + if (cf_options.table_factory->Name() != BlockBasedTableFactory().Name()) { + return Status::NotSupported( + "TTL is only supported in Block-Based Table format. "); + } + } + + if (cf_options.periodic_compaction_seconds > 0) { + if (db_options.max_open_files != -1) { + return Status::NotSupported( + "Periodic Compaction is only supported when files are always " + "kept open (set max_open_files = -1). "); + } + if (cf_options.table_factory->Name() != BlockBasedTableFactory().Name()) { + return Status::NotSupported( + "Periodic Compaction is only supported in " + "Block-Based Table format. "); + } + } + return s; +} + #ifndef ROCKSDB_LITE Status ColumnFamilyData::SetOptions( - const std::unordered_map& options_map) { + const DBOptions& db_options, + const std::unordered_map& options_map) { MutableCFOptions new_mutable_cf_options; Status s = GetMutableOptionsFromStrings(mutable_cf_options_, options_map, ioptions_.info_log, &new_mutable_cf_options); + if (s.ok()) { + ColumnFamilyOptions cf_options = + BuildColumnFamilyOptions(initial_cf_options_, new_mutable_cf_options); + s = ValidateOptions(db_options, cf_options); + } if (s.ok()) { mutable_cf_options_ = new_mutable_cf_options; mutable_cf_options_.RefreshDerivedOptions(ioptions_); diff --git a/db/column_family.h b/db/column_family.h index 655cb159261..8646b4fc197 100644 --- a/db/column_family.h +++ b/db/column_family.h @@ -338,9 +338,13 @@ class ColumnFamilyData { bool is_delete_range_supported() { return is_delete_range_supported_; } + // Validate CF options against DB options + static Status ValidateOptions(const DBOptions& db_options, + const ColumnFamilyOptions& cf_options); #ifndef ROCKSDB_LITE // REQUIRES: DB mutex held Status SetOptions( + const DBOptions& db_options, const std::unordered_map& options_map); #endif // ROCKSDB_LITE diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index 196e38f14fa..435dbbfe6bf 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -848,8 +848,9 @@ Status DBImpl::SetOptions( Status persist_options_status; SuperVersionContext sv_context(/* create_superversion */ true); { + auto db_options = GetDBOptions(); InstrumentedMutexLock l(&mutex_); - s = cfd->SetOptions(options_map); + s = cfd->SetOptions(db_options, options_map); if (s.ok()) { new_options = *cfd->GetLatestMutableCFOptions(); // Append new version to recompute compaction score. @@ -912,6 +913,25 @@ Status DBImpl::SetDBOptions( InstrumentedMutexLock l(&mutex_); s = GetMutableDBOptionsFromStrings(mutable_db_options_, options_map, &new_options); + if (new_options.bytes_per_sync == 0) { + new_options.bytes_per_sync = 1024 * 1024; + } + DBOptions new_db_options = + BuildDBOptions(immutable_db_options_, new_options); + if (s.ok()) { + s = ValidateOptions(new_db_options); + } + if (s.ok()) { + for (auto c : *versions_->GetColumnFamilySet()) { + if (!c->IsDropped()) { + auto cf_options = c->GetLatestCFOptions(); + s = ColumnFamilyData::ValidateOptions(new_db_options, cf_options); + if (!s.ok()) { + break; + } + } + } + } if (s.ok()) { if (new_options.max_background_compactions > mutable_db_options_.max_background_compactions) { @@ -956,15 +976,12 @@ Status DBImpl::SetDBOptions( : new_options.max_open_files - 10); wal_changed = mutable_db_options_.wal_bytes_per_sync != new_options.wal_bytes_per_sync; - if (new_options.bytes_per_sync == 0) { - new_options.bytes_per_sync = 1024 * 1024; - } mutable_db_options_ = new_options; - env_options_for_compaction_ = EnvOptions( - BuildDBOptions(immutable_db_options_, mutable_db_options_)); + env_options_for_compaction_ = EnvOptions(new_db_options); env_options_for_compaction_ = env_->OptimizeForCompactionTableWrite( env_options_for_compaction_, immutable_db_options_); versions_->ChangeEnvOptions(mutable_db_options_); + //TODO(xiez): clarify why apply optimize for read to write options env_options_for_compaction_ = env_->OptimizeForCompactionTableRead( env_options_for_compaction_, immutable_db_options_); env_options_for_compaction_.compaction_readahead_size = diff --git a/db/db_impl/db_impl.h b/db/db_impl/db_impl.h index c241a36dbc3..1f8f990e4f7 100644 --- a/db/db_impl/db_impl.h +++ b/db/db_impl/db_impl.h @@ -1500,6 +1500,13 @@ class DBImpl : public DB { Status CreateWAL(uint64_t log_file_num, uint64_t recycle_log_number, size_t preallocate_block_size, log::Writer** new_log); + // Validate self-consistency of DB options + static Status ValidateOptions(const DBOptions& db_options); + // Validate self-consistency of DB options and its consistency with cf options + static Status ValidateOptions( + const DBOptions& db_options, + const std::vector& column_families); + // table_cache_ provides its own synchronization std::shared_ptr table_cache_; diff --git a/db/db_impl/db_impl_open.cc b/db/db_impl/db_impl_open.cc index 5019221b5ca..2fc12746d7d 100644 --- a/db/db_impl/db_impl_open.cc +++ b/db/db_impl/db_impl_open.cc @@ -145,7 +145,6 @@ DBOptions SanitizeOptions(const std::string& dbname, const DBOptions& src) { } namespace { - Status SanitizeOptionsByTable( const DBOptions& db_opts, const std::vector& column_families) { @@ -158,52 +157,23 @@ Status SanitizeOptionsByTable( } return Status::OK(); } +} // namespace -static Status ValidateOptions( +Status DBImpl::ValidateOptions( const DBOptions& db_options, const std::vector& column_families) { Status s; - for (auto& cfd : column_families) { - s = CheckCompressionSupported(cfd.options); - if (s.ok() && db_options.allow_concurrent_memtable_write) { - s = CheckConcurrentWritesSupported(cfd.options); - } - if (s.ok()) { - s = CheckCFPathsSupported(db_options, cfd.options); - } + s = ColumnFamilyData::ValidateOptions(db_options, cfd.options); if (!s.ok()) { return s; } - - if (cfd.options.ttl > 0) { - if (db_options.max_open_files != -1) { - return Status::NotSupported( - "TTL is only supported when files are always " - "kept open (set max_open_files = -1). "); - } - if (cfd.options.table_factory->Name() != - BlockBasedTableFactory().Name()) { - return Status::NotSupported( - "TTL is only supported in Block-Based Table format. "); - } - } - - if (cfd.options.periodic_compaction_seconds > 0) { - if (db_options.max_open_files != -1) { - return Status::NotSupported( - "Periodic Compaction is only supported when files are always " - "kept open (set max_open_files = -1). "); - } - if (cfd.options.table_factory->Name() != - BlockBasedTableFactory().Name()) { - return Status::NotSupported( - "Periodic Compaction is only supported in " - "Block-Based Table format. "); - } - } } + s = ValidateOptions(db_options); + return s; +} +Status DBImpl::ValidateOptions(const DBOptions& db_options) { if (db_options.db_paths.size() > 4) { return Status::NotSupported( "More than four DB paths are not supported yet. "); @@ -241,7 +211,7 @@ static Status ValidateOptions( return Status::OK(); } -} // namespace + Status DBImpl::NewDB() { VersionEdit new_db; new_db.SetLogNumber(0); diff --git a/db/db_options_test.cc b/db/db_options_test.cc index a9c8d218235..bf33153284e 100644 --- a/db/db_options_test.cc +++ b/db/db_options_test.cc @@ -66,10 +66,10 @@ class DBOptionsTest : public DBTestBase { std::unordered_map GetRandomizedMutableCFOptionsMap( Random* rnd) { - Options options; + Options options = CurrentOptions(); options.env = env_; ImmutableDBOptions db_options(options); - test::RandomInitCFOptions(&options, rnd); + test::RandomInitCFOptions(&options, options, rnd); auto sanitized_options = SanitizeOptions(db_options, options); auto opt_map = GetMutableCFOptionsMap(sanitized_options); delete options.compaction_filter; diff --git a/db/db_test.cc b/db/db_test.cc index 4c4bd382ca8..27cf790ee57 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -4884,11 +4884,14 @@ TEST_F(DBTest, DynamicMiscOptions) { ASSERT_OK(dbfull()->TEST_GetLatestMutableCFOptions(handles_[0], &mutable_cf_options)); ASSERT_EQ(CompressionType::kNoCompression, mutable_cf_options.compression); + // Appveyor fails with: Compression type Snappy is not linked with the binary +#ifndef OS_WIN ASSERT_OK(dbfull()->SetOptions({{"compression", "kSnappyCompression"}})); ASSERT_OK(dbfull()->TEST_GetLatestMutableCFOptions(handles_[0], &mutable_cf_options)); ASSERT_EQ(CompressionType::kSnappyCompression, mutable_cf_options.compression); +#endif // Test paranoid_file_checks already done in db_block_cache_test ASSERT_OK( dbfull()->SetOptions(handles_[1], {{"paranoid_file_checks", "true"}})); diff --git a/options/options_test.cc b/options/options_test.cc index 429b607e4f9..1aa3bace7dd 100644 --- a/options/options_test.cc +++ b/options/options_test.cc @@ -842,7 +842,7 @@ TEST_F(OptionsTest, OptionsComposeDecompose) { Random rnd(301); test::RandomInitDBOptions(&base_db_opts, &rnd); - test::RandomInitCFOptions(&base_cf_opts, &rnd); + test::RandomInitCFOptions(&base_cf_opts, base_db_opts, &rnd); Options base_opts(base_db_opts, base_cf_opts); DBOptions new_db_opts(base_opts); @@ -854,11 +854,12 @@ TEST_F(OptionsTest, OptionsComposeDecompose) { } TEST_F(OptionsTest, ColumnFamilyOptionsSerialization) { + Options options; ColumnFamilyOptions base_opt, new_opt; Random rnd(302); // Phase 1: randomly assign base_opt // custom type options - test::RandomInitCFOptions(&base_opt, &rnd); + test::RandomInitCFOptions(&base_opt, options, &rnd); // Phase 2: obtain a string from base_opt std::string base_options_file_content; @@ -1521,7 +1522,7 @@ TEST_F(OptionsParserTest, DumpAndParse) { for (int c = 0; c < num_cf; ++c) { ColumnFamilyOptions cf_opt; Random cf_rnd(0xFB + c); - test::RandomInitCFOptions(&cf_opt, &cf_rnd); + test::RandomInitCFOptions(&cf_opt, base_db_opt, &cf_rnd); if (c < 4) { cf_opt.prefix_extractor.reset(test::RandomSliceTransform(&rnd, c)); } diff --git a/test_util/testutil.cc b/test_util/testutil.cc index 18e1a45bb36..4e37cde40d1 100644 --- a/test_util/testutil.cc +++ b/test_util/testutil.cc @@ -162,7 +162,11 @@ std::string RandomName(Random* rnd, const size_t len) { } CompressionType RandomCompressionType(Random* rnd) { - return static_cast(rnd->Uniform(6)); + auto ret = static_cast(rnd->Uniform(6)); + while (!CompressionTypeSupported(ret)) { + ret = static_cast((static_cast(ret) + 1) % 6); + } + return ret; } void RandomCompressionTypeVector(const size_t count, @@ -293,7 +297,8 @@ void RandomInitDBOptions(DBOptions* db_opt, Random* rnd) { db_opt->stats_dump_period_sec = rnd->Uniform(100000); } -void RandomInitCFOptions(ColumnFamilyOptions* cf_opt, Random* rnd) { +void RandomInitCFOptions(ColumnFamilyOptions* cf_opt, DBOptions& db_options, + Random* rnd) { cf_opt->compaction_style = (CompactionStyle)(rnd->Uniform(4)); // boolean options @@ -345,8 +350,10 @@ void RandomInitCFOptions(ColumnFamilyOptions* cf_opt, Random* rnd) { // uint64_t options static const uint64_t uint_max = static_cast(UINT_MAX); - cf_opt->ttl = uint_max + rnd->Uniform(10000); - cf_opt->periodic_compaction_seconds = uint_max + rnd->Uniform(10000); + cf_opt->ttl = + db_options.max_open_files == -1 ? uint_max + rnd->Uniform(10000) : 0; + cf_opt->periodic_compaction_seconds = + db_options.max_open_files == -1 ? uint_max + rnd->Uniform(10000) : 0; cf_opt->max_sequential_skip_in_iterations = uint_max + rnd->Uniform(10000); cf_opt->target_file_size_base = uint_max + rnd->Uniform(10000); cf_opt->max_compaction_bytes = diff --git a/test_util/testutil.h b/test_util/testutil.h index 7890ce5f511..bc0b2b07d5f 100644 --- a/test_util/testutil.h +++ b/test_util/testutil.h @@ -657,7 +657,7 @@ void RandomInitDBOptions(DBOptions* db_opt, Random* rnd); // Randomly initialize the given ColumnFamilyOptions // Note that the caller is responsible for releasing non-null // cf_opt->compaction_filter. -void RandomInitCFOptions(ColumnFamilyOptions* cf_opt, Random* rnd); +void RandomInitCFOptions(ColumnFamilyOptions* cf_opt, DBOptions&, Random* rnd); // A dummy merge operator which can change its name class ChanglingMergeOperator : public MergeOperator { diff --git a/utilities/options/options_util_test.cc b/utilities/options/options_util_test.cc index 5b8015152ff..8c71dbf5dc3 100644 --- a/utilities/options/options_util_test.cc +++ b/utilities/options/options_util_test.cc @@ -58,7 +58,7 @@ TEST_F(OptionsUtilTest, SaveAndLoad) { cf_names.push_back(i == 0 ? kDefaultColumnFamilyName : test::RandomName(&rnd_, 10)); cf_opts.emplace_back(); - test::RandomInitCFOptions(&cf_opts.back(), &rnd_); + test::RandomInitCFOptions(&cf_opts.back(), db_opt, &rnd_); } const std::string kFileName = "OPTIONS-123456"; @@ -82,7 +82,7 @@ TEST_F(OptionsUtilTest, SaveAndLoad) { cf_opts[i].table_factory.get(), loaded_cf_descs[i].options.table_factory.get())); } - test::RandomInitCFOptions(&cf_opts[i], &rnd_); + test::RandomInitCFOptions(&cf_opts[i], db_opt, &rnd_); ASSERT_NOK(RocksDBOptionsParser::VerifyCFOptions( cf_opts[i], loaded_cf_descs[i].options)); } diff --git a/utilities/transactions/write_prepared_txn_db.cc b/utilities/transactions/write_prepared_txn_db.cc index bf94d83d82b..e2a8fbbf20f 100644 --- a/utilities/transactions/write_prepared_txn_db.cc +++ b/utilities/transactions/write_prepared_txn_db.cc @@ -210,7 +210,7 @@ Status WritePreparedTxnDB::WriteInternal(const WriteOptions& write_options_orig, WriteBatch empty_batch; write_options.disableWAL = true; write_options.sync = false; - const size_t ONE_BATCH = 1; // Just to inc the seq + const size_t ONE_BATCH = 1; // Just to inc the seq s = db_impl_->WriteImpl(write_options, &empty_batch, nullptr, nullptr, no_log_ref, DISABLE_MEMTABLE, &seq_used, ONE_BATCH, &update_commit_map_with_prepare);