From 8efb5ffa2a7428e4bebe5f2dff97c7a7414a1fc5 Mon Sep 17 00:00:00 2001 From: Siying Dong Date: Thu, 23 Feb 2017 14:53:03 -0800 Subject: [PATCH] =?UTF-8?q?[rocksdb][PR]=20Remove=20option=20min=5Fpartial?= =?UTF-8?q?=5Fmerge=5Foperands=20and=20verify=5Fchecksums=5Fin=5Fcomp?= =?UTF-8?q?=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: …action The two options, min_partial_merge_operands and verify_checksums_in_compaction, are not seldom used. Remove them to reduce the total number of options. Also remove them from Java and C interface. Closes https://github.com/facebook/rocksdb/pull/1902 Differential Revision: D4601219 Pulled By: siying fbshipit-source-id: aad4cb2 --- HISTORY.md | 2 + db/builder.cc | 1 - db/c.cc | 10 -- db/compaction_iterator_test.cc | 2 +- db/compaction_job.cc | 1 - db/db_test.cc | 6 -- db/merge_helper.cc | 3 +- db/merge_helper.h | 3 - db/merge_helper_test.cc | 2 +- db/merge_test.cc | 32 +++---- db/version_set.cc | 3 +- include/rocksdb/c.h | 5 - include/rocksdb/merge_operator.h | 6 +- include/rocksdb/options.h | 16 ---- java/rocksjni/options.cc | 96 ------------------- .../java/org/rocksdb/ColumnFamilyOptions.java | 31 ------ .../rocksdb/ColumnFamilyOptionsInterface.java | 27 ------ .../rocksdb/MutableColumnFamilyOptions.java | 15 +-- .../MutableColumnFamilyOptionsInterface.java | 20 ---- java/src/main/java/org/rocksdb/Options.java | 31 ------ .../org/rocksdb/ColumnFamilyOptionsTest.java | 18 ---- .../MutableColumnFamilyOptionsTest.java | 10 +- .../test/java/org/rocksdb/OptionsTest.java | 18 ---- util/cf_options.cc | 4 - util/cf_options.h | 10 +- util/options.cc | 6 -- util/options_helper.cc | 5 - util/options_helper.h | 8 +- util/options_settable_test.cc | 2 - util/options_test.cc | 2 - util/testutil.cc | 2 - 31 files changed, 30 insertions(+), 367 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 40454d45c..37e7a3978 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -3,6 +3,8 @@ ### Public API Change * Remove disableDataSync option. * Remove timeout_hint_us option from WriteOptions. The option has been deprecated and has no effect since 3.13.0. +* Remove option min_partial_merge_operands. Partial merge operands will always be merged in flush or compaction if there are more than one. +* Remove option verify_checksums_in_compaction. Compaction will always verify checksum. ## 5.2.0 (02/08/2017) ### Public API Change diff --git a/db/builder.cc b/db/builder.cc index b06f1c4fc..2fbc93862 100644 --- a/db/builder.cc +++ b/db/builder.cc @@ -122,7 +122,6 @@ Status BuildTable( MergeHelper merge(env, internal_comparator.user_comparator(), ioptions.merge_operator, nullptr, ioptions.info_log, - mutable_cf_options.min_partial_merge_operands, true /* internal key corruption is not ok */, snapshots.empty() ? 0 : snapshots.back()); diff --git a/db/c.cc b/db/c.cc index 68ecdb52c..da6f2e5d5 100644 --- a/db/c.cc +++ b/db/c.cc @@ -1824,11 +1824,6 @@ void rocksdb_options_set_enable_write_thread_adaptive_yield( opt->rep.enable_write_thread_adaptive_yield = v; } -void rocksdb_options_set_verify_checksums_in_compaction( - rocksdb_options_t* opt, unsigned char v) { - opt->rep.verify_checksums_in_compaction = v; -} - void rocksdb_options_set_max_sequential_skip_in_iterations( rocksdb_options_t* opt, uint64_t v) { opt->rep.max_sequential_skip_in_iterations = v; @@ -1980,11 +1975,6 @@ void rocksdb_options_set_max_successive_merges( opt->rep.max_successive_merges = v; } -void rocksdb_options_set_min_partial_merge_operands( - rocksdb_options_t* opt, uint32_t v) { - opt->rep.min_partial_merge_operands = v; -} - void rocksdb_options_set_bloom_locality( rocksdb_options_t* opt, uint32_t v) { opt->rep.bloom_locality = v; diff --git a/db/compaction_iterator_test.cc b/db/compaction_iterator_test.cc index d06aac49d..65601e183 100644 --- a/db/compaction_iterator_test.cc +++ b/db/compaction_iterator_test.cc @@ -182,7 +182,7 @@ class CompactionIteratorTest : public testing::Test { } merge_helper_.reset(new MergeHelper(Env::Default(), cmp_, merge_op, filter, - nullptr, 0U, false, 0, 0, nullptr, + nullptr, false, 0, 0, nullptr, &shutting_down_)); iter_.reset(new LoggingForwardVectorIterator(ks, vs)); iter_->SeekToFirst(); diff --git a/db/compaction_job.cc b/db/compaction_job.cc index 5571312d3..8fc7e542c 100644 --- a/db/compaction_job.cc +++ b/db/compaction_job.cc @@ -721,7 +721,6 @@ void CompactionJob::ProcessKeyValueCompaction(SubcompactionState* sub_compact) { MergeHelper merge( env_, cfd->user_comparator(), cfd->ioptions()->merge_operator, compaction_filter, db_options_.info_log.get(), - mutable_cf_options->min_partial_merge_operands, false /* internal key corruption is expected */, existing_snapshots_.empty() ? 0 : existing_snapshots_.back(), compact_->compaction->level(), db_options_.statistics.get(), diff --git a/db/db_test.cc b/db/db_test.cc index 1c9087954..97f3726b2 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -4691,12 +4691,6 @@ TEST_F(DBTest, DynamicMiscOptions) { ASSERT_OK(dbfull()->TEST_GetLatestMutableCFOptions(handles_[1], &mutable_cf_options)); ASSERT_EQ(true, mutable_cf_options.report_bg_io_stats); - // Test min_partial_merge_operands - ASSERT_OK( - dbfull()->SetOptions(handles_[1], {{"min_partial_merge_operands", "4"}})); - ASSERT_OK(dbfull()->TEST_GetLatestMutableCFOptions(handles_[1], - &mutable_cf_options)); - ASSERT_EQ(4, mutable_cf_options.min_partial_merge_operands); // Test compression // sanity check ASSERT_OK(dbfull()->SetOptions({{"compression", "kNoCompression"}})); diff --git a/db/merge_helper.cc b/db/merge_helper.cc index 9e3d7d1d7..b798dfd0e 100644 --- a/db/merge_helper.cc +++ b/db/merge_helper.cc @@ -283,8 +283,7 @@ Status MergeHelper::MergeUntil(InternalIterator* iter, // Attempt to use the user's associative merge function to // merge the stacked merge operands into a single operand. s = Status::MergeInProgress(); - if (merge_context_.GetNumOperands() >= 2 && - merge_context_.GetNumOperands() >= min_partial_merge_operands_) { + if (merge_context_.GetNumOperands() >= 2) { bool merge_success = false; std::string merge_result; { diff --git a/db/merge_helper.h b/db/merge_helper.h index 455b5fb07..de825d7b6 100644 --- a/db/merge_helper.h +++ b/db/merge_helper.h @@ -32,7 +32,6 @@ class MergeHelper { MergeHelper(Env* env, const Comparator* user_comparator, const MergeOperator* user_merge_operator, const CompactionFilter* compaction_filter, Logger* logger, - unsigned min_partial_merge_operands, bool assert_valid_internal_key, SequenceNumber latest_snapshot, int level = 0, Statistics* stats = nullptr, const std::atomic* shutting_down = nullptr) @@ -42,7 +41,6 @@ class MergeHelper { compaction_filter_(compaction_filter), shutting_down_(shutting_down), logger_(logger), - min_partial_merge_operands_(min_partial_merge_operands), assert_valid_internal_key_(assert_valid_internal_key), latest_snapshot_(latest_snapshot), level_(level), @@ -156,7 +154,6 @@ class MergeHelper { const CompactionFilter* compaction_filter_; const std::atomic* shutting_down_; Logger* logger_; - unsigned min_partial_merge_operands_; bool assert_valid_internal_key_; // enforce no internal key corruption? SequenceNumber latest_snapshot_; int level_; diff --git a/db/merge_helper_test.cc b/db/merge_helper_test.cc index e468f7ed2..c6ceec9cf 100644 --- a/db/merge_helper_test.cc +++ b/db/merge_helper_test.cc @@ -28,7 +28,7 @@ class MergeHelperTest : public testing::Test { iter_->SeekToFirst(); merge_helper_.reset(new MergeHelper(env_, BytewiseComparator(), merge_op_.get(), filter_.get(), nullptr, - 2U, false, latest_snapshot)); + false, latest_snapshot)); return merge_helper_->MergeUntil(iter_.get(), nullptr /* range_del_agg */, stop_before, at_bottom); } diff --git a/db/merge_test.cc b/db/merge_test.cc index 020f33ba6..5d8be99f2 100644 --- a/db/merge_test.cc +++ b/db/merge_test.cc @@ -76,14 +76,12 @@ class CountMergeOperator : public AssociativeMergeOperator { namespace { std::shared_ptr OpenDb(const std::string& dbname, const bool ttl = false, - const size_t max_successive_merges = 0, - const uint32_t min_partial_merge_operands = 2) { + const size_t max_successive_merges = 0) { DB* db; Options options; options.create_if_missing = true; options.merge_operator = std::make_shared(); options.max_successive_merges = max_successive_merges; - options.min_partial_merge_operands = min_partial_merge_operands; Status s; DestroyDB(dbname, Options()); // DBWithTTL is not supported in ROCKSDB_LITE @@ -448,20 +446,20 @@ void runTest(int argc, const std::string& dbname, const bool use_ttl = false) { { std::cout << "Test Partial-Merge\n"; size_t max_merge = 100; - for (uint32_t min_merge = 5; min_merge < 25; min_merge += 5) { - for (uint32_t count = min_merge - 1; count <= min_merge + 1; count++) { - auto db = OpenDb(dbname, use_ttl, max_merge, min_merge); - MergeBasedCounters counters(db, 0); - testPartialMerge(&counters, db.get(), max_merge, min_merge, count); - DestroyDB(dbname, Options()); - } - { - auto db = OpenDb(dbname, use_ttl, max_merge, min_merge); - MergeBasedCounters counters(db, 0); - testPartialMerge(&counters, db.get(), max_merge, min_merge, - min_merge * 10); - DestroyDB(dbname, Options()); - } + // Min merge is hard-coded to 2. + uint32_t min_merge = 2; + for (uint32_t count = min_merge - 1; count <= min_merge + 1; count++) { + auto db = OpenDb(dbname, use_ttl, max_merge); + MergeBasedCounters counters(db, 0); + testPartialMerge(&counters, db.get(), max_merge, min_merge, count); + DestroyDB(dbname, Options()); + } + { + auto db = OpenDb(dbname, use_ttl, max_merge); + MergeBasedCounters counters(db, 0); + testPartialMerge(&counters, db.get(), max_merge, min_merge, + min_merge * 10); + DestroyDB(dbname, Options()); } } diff --git a/db/version_set.cc b/db/version_set.cc index 6380bcfe3..31606c396 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -3431,8 +3431,7 @@ InternalIterator* VersionSet::MakeInputIterator( const Compaction* c, RangeDelAggregator* range_del_agg) { auto cfd = c->column_family_data(); ReadOptions read_options; - read_options.verify_checksums = - c->mutable_cf_options()->verify_checksums_in_compaction; + read_options.verify_checksums = true; read_options.fill_cache = false; if (c->ShouldFormSubcompactions()) { read_options.total_order_seek = true; diff --git a/include/rocksdb/c.h b/include/rocksdb/c.h index 4193c724c..7dd004558 100644 --- a/include/rocksdb/c.h +++ b/include/rocksdb/c.h @@ -700,9 +700,6 @@ extern ROCKSDB_LIBRARY_API void rocksdb_options_set_enable_write_thread_adaptive_yield(rocksdb_options_t*, unsigned char); extern ROCKSDB_LIBRARY_API void -rocksdb_options_set_verify_checksums_in_compaction(rocksdb_options_t*, - unsigned char); -extern ROCKSDB_LIBRARY_API void rocksdb_options_set_max_sequential_skip_in_iterations(rocksdb_options_t*, uint64_t); extern ROCKSDB_LIBRARY_API void rocksdb_options_set_disable_auto_compactions( @@ -735,8 +732,6 @@ extern ROCKSDB_LIBRARY_API void rocksdb_options_set_memtable_huge_page_size( extern ROCKSDB_LIBRARY_API void rocksdb_options_set_max_successive_merges( rocksdb_options_t*, size_t); -extern ROCKSDB_LIBRARY_API void rocksdb_options_set_min_partial_merge_operands( - rocksdb_options_t*, uint32_t); extern ROCKSDB_LIBRARY_API void rocksdb_options_set_bloom_locality( rocksdb_options_t*, uint32_t); extern ROCKSDB_LIBRARY_API void rocksdb_options_set_inplace_update_support( diff --git a/include/rocksdb/merge_operator.h b/include/rocksdb/merge_operator.h index b3ca013b7..6ec34fe3e 100644 --- a/include/rocksdb/merge_operator.h +++ b/include/rocksdb/merge_operator.h @@ -165,10 +165,8 @@ class MergeOperator { // // The string that new_value is pointing to will be empty. // - // The PartialMergeMulti function will be called only when the list of - // operands are long enough. The minimum amount of operands that will be - // passed to the function are specified by the "min_partial_merge_operands" - // option. + // The PartialMergeMulti function will be called when there are at least two + // operands. // // In the default implementation, PartialMergeMulti will invoke PartialMerge // multiple times, where each time it only merges two operands. Developers diff --git a/include/rocksdb/options.h b/include/rocksdb/options.h index 0f8635343..2f703dd4c 100644 --- a/include/rocksdb/options.h +++ b/include/rocksdb/options.h @@ -615,13 +615,6 @@ struct ColumnFamilyOptions { // Default: kByCompensatedSize CompactionPri compaction_pri = kByCompensatedSize; - // If true, compaction will verify checksum on every read that happens - // as part of compaction - // - // Default: true - // - // Dynamically changeable through SetOptions() API - bool verify_checksums_in_compaction = true; // The options needed to support Universal Style compactions CompactionOptionsUniversal compaction_options_universal; @@ -798,15 +791,6 @@ struct ColumnFamilyOptions { // Dynamically changeable through SetOptions() API size_t max_successive_merges = 0; - // The number of partial merge operands to accumulate before partial - // merge will be performed. Partial merge will not be called - // if the list of values to merge is less than min_partial_merge_operands. - // - // If min_partial_merge_operands < 2, then it will be treated as 2. - // - // Default: 2 - uint32_t min_partial_merge_operands = 2; - // This flag specifies that the implementation should optimize the filters // mainly for cases where keys are found rather than also optimize for keys // missed. This would be used in cases where the application knows that diff --git a/java/rocksjni/options.cc b/java/rocksjni/options.cc index 83af11e95..117b2bc4e 100644 --- a/java/rocksjni/options.cc +++ b/java/rocksjni/options.cc @@ -1707,30 +1707,6 @@ void Java_org_rocksdb_Options_setPurgeRedundantKvsWhileFlush( static_cast(jpurge_redundant_kvs_while_flush); } -/* - * Class: org_rocksdb_Options - * Method: verifyChecksumsInCompaction - * Signature: (J)Z - */ -jboolean Java_org_rocksdb_Options_verifyChecksumsInCompaction( - JNIEnv* env, jobject jobj, jlong jhandle) { - return reinterpret_cast( - jhandle)->verify_checksums_in_compaction; -} - -/* - * Class: org_rocksdb_Options - * Method: setVerifyChecksumsInCompaction - * Signature: (JZ)V - */ -void Java_org_rocksdb_Options_setVerifyChecksumsInCompaction( - JNIEnv* env, jobject jobj, jlong jhandle, - jboolean jverify_checksums_in_compaction) { - reinterpret_cast( - jhandle)->verify_checksums_in_compaction = - static_cast(jverify_checksums_in_compaction); -} - /* * Class: org_rocksdb_Options * Method: maxSequentialSkipInIterations @@ -1882,30 +1858,6 @@ void Java_org_rocksdb_Options_setMaxSuccessiveMerges( } } -/* - * Class: org_rocksdb_Options - * Method: minPartialMergeOperands - * Signature: (J)I - */ -jint Java_org_rocksdb_Options_minPartialMergeOperands( - JNIEnv* env, jobject jobj, jlong jhandle) { - return reinterpret_cast( - jhandle)->min_partial_merge_operands; -} - -/* - * Class: org_rocksdb_Options - * Method: setMinPartialMergeOperands - * Signature: (JI)V - */ -void Java_org_rocksdb_Options_setMinPartialMergeOperands( - JNIEnv* env, jobject jobj, jlong jhandle, - jint jmin_partial_merge_operands) { - reinterpret_cast( - jhandle)->min_partial_merge_operands = - static_cast(jmin_partial_merge_operands); -} - /* * Class: org_rocksdb_Options * Method: optimizeFiltersForHits @@ -3013,30 +2965,6 @@ void Java_org_rocksdb_ColumnFamilyOptions_setPurgeRedundantKvsWhileFlush( static_cast(jpurge_redundant_kvs_while_flush); } -/* - * Class: org_rocksdb_ColumnFamilyOptions - * Method: verifyChecksumsInCompaction - * Signature: (J)Z - */ -jboolean Java_org_rocksdb_ColumnFamilyOptions_verifyChecksumsInCompaction( - JNIEnv* env, jobject jobj, jlong jhandle) { - return reinterpret_cast( - jhandle)->verify_checksums_in_compaction; -} - -/* - * Class: org_rocksdb_ColumnFamilyOptions - * Method: setVerifyChecksumsInCompaction - * Signature: (JZ)V - */ -void Java_org_rocksdb_ColumnFamilyOptions_setVerifyChecksumsInCompaction( - JNIEnv* env, jobject jobj, jlong jhandle, - jboolean jverify_checksums_in_compaction) { - reinterpret_cast( - jhandle)->verify_checksums_in_compaction = - static_cast(jverify_checksums_in_compaction); -} - /* * Class: org_rocksdb_ColumnFamilyOptions * Method: maxSequentialSkipInIterations @@ -3189,30 +3117,6 @@ void Java_org_rocksdb_ColumnFamilyOptions_setMaxSuccessiveMerges( } } -/* - * Class: org_rocksdb_ColumnFamilyOptions - * Method: minPartialMergeOperands - * Signature: (J)I - */ -jint Java_org_rocksdb_ColumnFamilyOptions_minPartialMergeOperands( - JNIEnv* env, jobject jobj, jlong jhandle) { - return reinterpret_cast( - jhandle)->min_partial_merge_operands; -} - -/* - * Class: org_rocksdb_ColumnFamilyOptions - * Method: setMinPartialMergeOperands - * Signature: (JI)V - */ -void Java_org_rocksdb_ColumnFamilyOptions_setMinPartialMergeOperands( - JNIEnv* env, jobject jobj, jlong jhandle, - jint jmin_partial_merge_operands) { - reinterpret_cast( - jhandle)->min_partial_merge_operands = - static_cast(jmin_partial_merge_operands); -} - /* * Class: org_rocksdb_ColumnFamilyOptions * Method: optimizeFiltersForHits diff --git a/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java b/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java index 254da9d32..5d4c63485 100644 --- a/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java +++ b/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java @@ -473,19 +473,6 @@ public class ColumnFamilyOptions extends RocksObject return maxTableFilesSizeFIFO(nativeHandle_); } - @Override - public ColumnFamilyOptions setVerifyChecksumsInCompaction( - final boolean verifyChecksumsInCompaction) { - setVerifyChecksumsInCompaction( - nativeHandle_, verifyChecksumsInCompaction); - return this; - } - - @Override - public boolean verifyChecksumsInCompaction() { - return verifyChecksumsInCompaction(nativeHandle_); - } - @Override public ColumnFamilyOptions setMaxSequentialSkipInIterations( final long maxSequentialSkipInIterations) { @@ -586,18 +573,6 @@ public class ColumnFamilyOptions extends RocksObject return maxSuccessiveMerges(nativeHandle_); } - @Override - public ColumnFamilyOptions setMinPartialMergeOperands( - final int minPartialMergeOperands) { - setMinPartialMergeOperands(nativeHandle_, minPartialMergeOperands); - return this; - } - - @Override - public int minPartialMergeOperands() { - return minPartialMergeOperands(nativeHandle_); - } - @Override public ColumnFamilyOptions setOptimizeFiltersForHits( final boolean optimizeFiltersForHits) { @@ -802,9 +777,6 @@ public class ColumnFamilyOptions extends RocksObject private native void setPurgeRedundantKvsWhileFlush( long handle, boolean purgeRedundantKvsWhileFlush); private native boolean purgeRedundantKvsWhileFlush(long handle); - private native void setVerifyChecksumsInCompaction( - long handle, boolean verifyChecksumsInCompaction); - private native boolean verifyChecksumsInCompaction(long handle); private native void setMaxSequentialSkipInIterations( long handle, long maxSequentialSkipInIterations); private native long maxSequentialSkipInIterations(long handle); @@ -829,9 +801,6 @@ public class ColumnFamilyOptions extends RocksObject long handle, long maxSuccessiveMerges) throws IllegalArgumentException; private native long maxSuccessiveMerges(long handle); - private native void setMinPartialMergeOperands( - long handle, int minPartialMergeOperands); - private native int minPartialMergeOperands(long handle); private native void setOptimizeFiltersForHits(long handle, boolean optimizeFiltersForHits); private native boolean optimizeFiltersForHits(long handle); diff --git a/java/src/main/java/org/rocksdb/ColumnFamilyOptionsInterface.java b/java/src/main/java/org/rocksdb/ColumnFamilyOptionsInterface.java index 284d2f85b..4d429b0e7 100644 --- a/java/src/main/java/org/rocksdb/ColumnFamilyOptionsInterface.java +++ b/java/src/main/java/org/rocksdb/ColumnFamilyOptionsInterface.java @@ -699,33 +699,6 @@ public interface ColumnFamilyOptionsInterface { */ int bloomLocality(); - /** - * The number of partial merge operands to accumulate before partial - * merge will be performed. Partial merge will not be called - * if the list of values to merge is less than min_partial_merge_operands. - * - * If min_partial_merge_operands < 2, then it will be treated as 2. - * - * Default: 2 - * - * @param minPartialMergeOperands min partial merge operands - * @return the reference to the current option. - */ - Object setMinPartialMergeOperands(int minPartialMergeOperands); - - /** - * The number of partial merge operands to accumulate before partial - * merge will be performed. Partial merge will not be called - * if the list of values to merge is less than min_partial_merge_operands. - * - * If min_partial_merge_operands < 2, then it will be treated as 2. - * - * Default: 2 - * - * @return min partial merge operands - */ - int minPartialMergeOperands(); - /** *

This flag specifies that the implementation should optimize the filters * mainly for cases where keys are found rather than also optimize for keys diff --git a/java/src/main/java/org/rocksdb/MutableColumnFamilyOptions.java b/java/src/main/java/org/rocksdb/MutableColumnFamilyOptions.java index 8e86d7f8d..7dc477908 100644 --- a/java/src/main/java/org/rocksdb/MutableColumnFamilyOptions.java +++ b/java/src/main/java/org/rocksdb/MutableColumnFamilyOptions.java @@ -148,8 +148,7 @@ public class MutableColumnFamilyOptions { target_file_size_multiplier(ValueType.INT), max_bytes_for_level_base(ValueType.LONG), max_bytes_for_level_multiplier(ValueType.INT), - max_bytes_for_level_multiplier_additional(ValueType.INT_ARRAY), - verify_checksums_in_compaction(ValueType.BOOLEAN); + max_bytes_for_level_multiplier_additional(ValueType.INT_ARRAY); private final ValueType valueType; CompactionOption(final ValueType valueType) { @@ -866,18 +865,6 @@ public class MutableColumnFamilyOptions { CompactionOption.max_bytes_for_level_multiplier_additional); } - @Override - public MutableColumnFamilyOptionsBuilder setVerifyChecksumsInCompaction( - final boolean verifyChecksumsInCompaction) { - return setBoolean(CompactionOption.verify_checksums_in_compaction, - verifyChecksumsInCompaction); - } - - @Override - public boolean verifyChecksumsInCompaction() { - return getBoolean(CompactionOption.verify_checksums_in_compaction); - } - @Override public MutableColumnFamilyOptionsBuilder setMaxSequentialSkipInIterations( final long maxSequentialSkipInIterations) { diff --git a/java/src/main/java/org/rocksdb/MutableColumnFamilyOptionsInterface.java b/java/src/main/java/org/rocksdb/MutableColumnFamilyOptionsInterface.java index d1d699360..7a128c7eb 100644 --- a/java/src/main/java/org/rocksdb/MutableColumnFamilyOptionsInterface.java +++ b/java/src/main/java/org/rocksdb/MutableColumnFamilyOptionsInterface.java @@ -543,26 +543,6 @@ public interface MutableColumnFamilyOptionsInterface { */ int[] maxBytesForLevelMultiplierAdditional(); - /** - * If true, compaction will verify checksum on every read that happens - * as part of compaction - * Default: true - * - * @param verifyChecksumsInCompaction true if compaction verifies - * checksum on every read. - * @return the reference to the current option. - */ - MutableColumnFamilyOptionsInterface setVerifyChecksumsInCompaction( - boolean verifyChecksumsInCompaction); - - /** - * If true, compaction will verify checksum on every read that happens - * as part of compaction - * Default: true - * - * @return true if compaction verifies checksum on every read. - */ - boolean verifyChecksumsInCompaction(); /** * An iteration->Next() sequentially skips over keys with the same diff --git a/java/src/main/java/org/rocksdb/Options.java b/java/src/main/java/org/rocksdb/Options.java index 13d308fde..924bad482 100644 --- a/java/src/main/java/org/rocksdb/Options.java +++ b/java/src/main/java/org/rocksdb/Options.java @@ -997,19 +997,6 @@ public class Options extends RocksObject return this; } - @Override - public boolean verifyChecksumsInCompaction() { - return verifyChecksumsInCompaction(nativeHandle_); - } - - @Override - public Options setVerifyChecksumsInCompaction( - final boolean verifyChecksumsInCompaction) { - setVerifyChecksumsInCompaction( - nativeHandle_, verifyChecksumsInCompaction); - return this; - } - @Override public long maxSequentialSkipInIterations() { return maxSequentialSkipInIterations(nativeHandle_); @@ -1092,18 +1079,6 @@ public class Options extends RocksObject return this; } - @Override - public int minPartialMergeOperands() { - return minPartialMergeOperands(nativeHandle_); - } - - @Override - public Options setMinPartialMergeOperands( - final int minPartialMergeOperands) { - setMinPartialMergeOperands(nativeHandle_, minPartialMergeOperands); - return this; - } - @Override public Options setOptimizeFiltersForHits( final boolean optimizeFiltersForHits) { @@ -1406,9 +1381,6 @@ public class Options extends RocksObject private native void setPurgeRedundantKvsWhileFlush( long handle, boolean purgeRedundantKvsWhileFlush); private native boolean purgeRedundantKvsWhileFlush(long handle); - private native void setVerifyChecksumsInCompaction( - long handle, boolean verifyChecksumsInCompaction); - private native boolean verifyChecksumsInCompaction(long handle); private native void setMaxSequentialSkipInIterations( long handle, long maxSequentialSkipInIterations); private native long maxSequentialSkipInIterations(long handle); @@ -1433,9 +1405,6 @@ public class Options extends RocksObject long handle, long maxSuccessiveMerges) throws IllegalArgumentException; private native long maxSuccessiveMerges(long handle); - private native void setMinPartialMergeOperands( - long handle, int minPartialMergeOperands); - private native int minPartialMergeOperands(long handle); private native void setOptimizeFiltersForHits(long handle, boolean optimizeFiltersForHits); private native boolean optimizeFiltersForHits(long handle); diff --git a/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java b/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java index e0dbe3011..59962693a 100644 --- a/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java +++ b/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java @@ -297,15 +297,6 @@ public class ColumnFamilyOptionsTest { } } - @Test - public void verifyChecksumsInCompaction() { - try (final ColumnFamilyOptions opt = new ColumnFamilyOptions()) { - final boolean boolValue = rand.nextBoolean(); - opt.setVerifyChecksumsInCompaction(boolValue); - assertThat(opt.verifyChecksumsInCompaction()).isEqualTo(boolValue); - } - } - @Test public void maxSequentialSkipInIterations() { try (final ColumnFamilyOptions opt = new ColumnFamilyOptions()) { @@ -369,15 +360,6 @@ public class ColumnFamilyOptionsTest { } } - @Test - public void minPartialMergeOperands() { - try (final ColumnFamilyOptions opt = new ColumnFamilyOptions()) { - final int intValue = rand.nextInt(); - opt.setMinPartialMergeOperands(intValue); - assertThat(opt.minPartialMergeOperands()).isEqualTo(intValue); - } - } - @Test public void optimizeFiltersForHits() { try (final ColumnFamilyOptions opt = new ColumnFamilyOptions()) { diff --git a/java/src/test/java/org/rocksdb/MutableColumnFamilyOptionsTest.java b/java/src/test/java/org/rocksdb/MutableColumnFamilyOptionsTest.java index a296d4adb..eb7b3ebc3 100644 --- a/java/src/test/java/org/rocksdb/MutableColumnFamilyOptionsTest.java +++ b/java/src/test/java/org/rocksdb/MutableColumnFamilyOptionsTest.java @@ -21,13 +21,11 @@ public class MutableColumnFamilyOptionsTest { .setWriteBufferSize(10) .setInplaceUpdateNumLocks(5) .setDisableAutoCompactions(true) - .setVerifyChecksumsInCompaction(false) .setParanoidFileChecks(true); assertThat(builder.writeBufferSize()).isEqualTo(10); assertThat(builder.inplaceUpdateNumLocks()).isEqualTo(5); assertThat(builder.disableAutoCompactions()).isEqualTo(true); - assertThat(builder.verifyChecksumsInCompaction()).isEqualTo(false); assertThat(builder.paranoidFileChecks()).isEqualTo(true); } @@ -66,21 +64,18 @@ public class MutableColumnFamilyOptionsTest { .setWriteBufferSize(10) .setInplaceUpdateNumLocks(5) .setDisableAutoCompactions(true) - .setVerifyChecksumsInCompaction(false) .setParanoidFileChecks(true) .build() .toString(); assertThat(str).isEqualTo("write_buffer_size=10;inplace_update_num_locks=5;" - + "disable_auto_compactions=true;verify_checksums_in_compaction=false;" - + "paranoid_file_checks=true"); + + "disable_auto_compactions=true;paranoid_file_checks=true"); } @Test public void mutableColumnFamilyOptions_parse() { final String str = "write_buffer_size=10;inplace_update_num_locks=5;" - + "disable_auto_compactions=true;verify_checksums_in_compaction=false;" - + "paranoid_file_checks=true"; + + "disable_auto_compactions=true;paranoid_file_checks=true"; final MutableColumnFamilyOptionsBuilder builder = MutableColumnFamilyOptions.parse(str); @@ -88,7 +83,6 @@ public class MutableColumnFamilyOptionsTest { assertThat(builder.writeBufferSize()).isEqualTo(10); assertThat(builder.inplaceUpdateNumLocks()).isEqualTo(5); assertThat(builder.disableAutoCompactions()).isEqualTo(true); - assertThat(builder.verifyChecksumsInCompaction()).isEqualTo(false); assertThat(builder.paranoidFileChecks()).isEqualTo(true); } } diff --git a/java/src/test/java/org/rocksdb/OptionsTest.java b/java/src/test/java/org/rocksdb/OptionsTest.java index 26380451c..084a77af0 100644 --- a/java/src/test/java/org/rocksdb/OptionsTest.java +++ b/java/src/test/java/org/rocksdb/OptionsTest.java @@ -260,15 +260,6 @@ public class OptionsTest { } } - @Test - public void verifyChecksumsInCompaction() { - try (final Options opt = new Options()) { - final boolean boolValue = rand.nextBoolean(); - opt.setVerifyChecksumsInCompaction(boolValue); - assertThat(opt.verifyChecksumsInCompaction()).isEqualTo(boolValue); - } - } - @Test public void maxSequentialSkipInIterations() { try (final Options opt = new Options()) { @@ -332,15 +323,6 @@ public class OptionsTest { } } - @Test - public void minPartialMergeOperands() { - try (final Options opt = new Options()) { - final int intValue = rand.nextInt(); - opt.setMinPartialMergeOperands(intValue); - assertThat(opt.minPartialMergeOperands()).isEqualTo(intValue); - } - } - @Test public void optimizeFiltersForHits() { try (final Options opt = new Options()) { diff --git a/util/cf_options.cc b/util/cf_options.cc index a6da3e92e..a25613636 100644 --- a/util/cf_options.cc +++ b/util/cf_options.cc @@ -158,8 +158,6 @@ void MutableCFOptions::Dump(Logger* log) const { } Log(log, "max_bytes_for_level_multiplier_additional: %s", result.c_str()); - Log(log, " verify_checksums_in_compaction: %d", - verify_checksums_in_compaction); Log(log, " max_sequential_skip_in_iterations: %" PRIu64, max_sequential_skip_in_iterations); Log(log, " paranoid_file_checks: %d", @@ -167,8 +165,6 @@ void MutableCFOptions::Dump(Logger* log) const { Log(log, " report_bg_io_stats: %d", report_bg_io_stats); Log(log, " compression: %d", static_cast(compression)); - Log(log, " min_partial_merge_operands: %" PRIu32, - min_partial_merge_operands); } } // namespace rocksdb diff --git a/util/cf_options.h b/util/cf_options.h index 0354a0175..5d768c12f 100644 --- a/util/cf_options.h +++ b/util/cf_options.h @@ -145,13 +145,11 @@ struct MutableCFOptions { max_bytes_for_level_multiplier(options.max_bytes_for_level_multiplier), max_bytes_for_level_multiplier_additional( options.max_bytes_for_level_multiplier_additional), - verify_checksums_in_compaction(options.verify_checksums_in_compaction), max_sequential_skip_in_iterations( options.max_sequential_skip_in_iterations), paranoid_file_checks(options.paranoid_file_checks), report_bg_io_stats(options.report_bg_io_stats), - compression(options.compression), - min_partial_merge_operands(options.min_partial_merge_operands) { + compression(options.compression) { RefreshDerivedOptions(options.num_levels, options.compaction_style); } @@ -174,12 +172,10 @@ struct MutableCFOptions { target_file_size_multiplier(0), max_bytes_for_level_base(0), max_bytes_for_level_multiplier(0), - verify_checksums_in_compaction(false), max_sequential_skip_in_iterations(0), paranoid_file_checks(false), report_bg_io_stats(false), - compression(Snappy_Supported() ? kSnappyCompression : kNoCompression), - min_partial_merge_operands(2) {} + compression(Snappy_Supported() ? kSnappyCompression : kNoCompression) {} // Must be called after any change to MutableCFOptions void RefreshDerivedOptions(int num_levels, CompactionStyle compaction_style); @@ -222,14 +218,12 @@ struct MutableCFOptions { uint64_t max_bytes_for_level_base; double max_bytes_for_level_multiplier; std::vector max_bytes_for_level_multiplier_additional; - bool verify_checksums_in_compaction; // Misc options uint64_t max_sequential_skip_in_iterations; bool paranoid_file_checks; bool report_bg_io_stats; CompressionType compression; - uint32_t min_partial_merge_operands; // Derived options // Per-level target file size. diff --git a/util/options.cc b/util/options.cc index 9242ad541..b16d0feb4 100644 --- a/util/options.cc +++ b/util/options.cc @@ -85,7 +85,6 @@ ColumnFamilyOptions::ColumnFamilyOptions(const Options& options) purge_redundant_kvs_while_flush(options.purge_redundant_kvs_while_flush), compaction_style(options.compaction_style), compaction_pri(options.compaction_pri), - verify_checksums_in_compaction(options.verify_checksums_in_compaction), compaction_options_universal(options.compaction_options_universal), compaction_options_fifo(options.compaction_options_fifo), max_sequential_skip_in_iterations( @@ -104,7 +103,6 @@ ColumnFamilyOptions::ColumnFamilyOptions(const Options& options) options.memtable_insert_with_hint_prefix_extractor), bloom_locality(options.bloom_locality), max_successive_merges(options.max_successive_merges), - min_partial_merge_operands(options.min_partial_merge_operands), optimize_filters_for_hits(options.optimize_filters_for_hits), paranoid_file_checks(options.paranoid_file_checks), force_consistency_checks(options.force_consistency_checks), @@ -282,8 +280,6 @@ void ColumnFamilyOptions::Dump(Logger* log) const { rate_limit_delay_max_milliseconds); Header(log, " Options.disable_auto_compactions: %d", disable_auto_compactions); - Header(log, " Options.verify_checksums_in_compaction: %d", - verify_checksums_in_compaction); const auto& it_compaction_style = compaction_style_to_string.find(compaction_style); @@ -335,8 +331,6 @@ void ColumnFamilyOptions::Dump(Logger* log) const { Header(log, " Options.inplace_update_num_locks: %" ROCKSDB_PRIszt, inplace_update_num_locks); - Header(log, " Options.min_partial_merge_operands: %u", - min_partial_merge_operands); // TODO: easier config for bloom (maybe based on avg key/value size) Header(log, " Options.memtable_prefix_bloom_size_ratio: %f", memtable_prefix_bloom_size_ratio); diff --git a/util/options_helper.cc b/util/options_helper.cc index 2aa071bd8..a4637fa90 100644 --- a/util/options_helper.cc +++ b/util/options_helper.cc @@ -163,17 +163,12 @@ ColumnFamilyOptions BuildColumnFamilyOptions( cf_opts.max_bytes_for_level_multiplier_additional.emplace_back(value); } - cf_opts.verify_checksums_in_compaction = - mutable_cf_options.verify_checksums_in_compaction; - // Misc options cf_opts.max_sequential_skip_in_iterations = mutable_cf_options.max_sequential_skip_in_iterations; cf_opts.paranoid_file_checks = mutable_cf_options.paranoid_file_checks; cf_opts.report_bg_io_stats = mutable_cf_options.report_bg_io_stats; cf_opts.compression = mutable_cf_options.compression; - cf_opts.min_partial_merge_operands = - mutable_cf_options.min_partial_merge_operands; cf_opts.table_factory = options.table_factory; // TODO(yhchiang): find some way to handle the following derived options diff --git a/util/options_helper.h b/util/options_helper.h index bd938c573..df0eafe37 100644 --- a/util/options_helper.h +++ b/util/options_helper.h @@ -411,9 +411,7 @@ static std::unordered_map cf_options_type_info = { {offsetof(struct ColumnFamilyOptions, purge_redundant_kvs_while_flush), OptionType::kBoolean, OptionVerificationType::kNormal, false, 0}}, {"verify_checksums_in_compaction", - {offsetof(struct ColumnFamilyOptions, verify_checksums_in_compaction), - OptionType::kBoolean, OptionVerificationType::kNormal, true, - offsetof(struct MutableCFOptions, verify_checksums_in_compaction)}}, + {0, OptionType::kBoolean, OptionVerificationType::kDeprecated, true, 0}}, {"soft_pending_compaction_bytes_limit", {offsetof(struct ColumnFamilyOptions, soft_pending_compaction_bytes_limit), OptionType::kUInt64T, OptionVerificationType::kNormal, true, @@ -501,9 +499,7 @@ static std::unordered_map cf_options_type_info = { {"memtable_prefix_bloom_probes", {0, OptionType::kUInt32T, OptionVerificationType::kDeprecated, true, 0}}, {"min_partial_merge_operands", - {offsetof(struct ColumnFamilyOptions, min_partial_merge_operands), - OptionType::kUInt32T, OptionVerificationType::kNormal, true, - offsetof(struct MutableCFOptions, min_partial_merge_operands)}}, + {0, OptionType::kUInt32T, OptionVerificationType::kDeprecated, true, 0}}, {"max_bytes_for_level_base", {offsetof(struct ColumnFamilyOptions, max_bytes_for_level_base), OptionType::kUInt64T, OptionVerificationType::kNormal, true, diff --git a/util/options_settable_test.cc b/util/options_settable_test.cc index 9b0cedc4b..374a2c6a8 100644 --- a/util/options_settable_test.cc +++ b/util/options_settable_test.cc @@ -411,7 +411,6 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) { "memtable_factory=SkipListFactory;" "compression=kNoCompression;" "bottommost_compression=kDisableCompressionOption;" - "min_partial_merge_operands=7576;" "level0_stop_writes_trigger=33;" "num_levels=99;" "level0_slowdown_writes_trigger=22;" @@ -420,7 +419,6 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) { "soft_rate_limit=530.615385;" "soft_pending_compaction_bytes_limit=0;" "max_write_buffer_number_to_maintain=84;" - "verify_checksums_in_compaction=false;" "merge_operator=aabcxehazrMergeOperator;" "memtable_prefix_bloom_size_ratio=0.4642;" "memtable_insert_with_hint_prefix_extractor=rocksdb.CappedPrefix.13;" diff --git a/util/options_test.cc b/util/options_test.cc index 3d0b997c2..ccd3a3322 100644 --- a/util/options_test.cc +++ b/util/options_test.cc @@ -174,7 +174,6 @@ TEST_F(OptionsTest, GetOptionsFromMapTest) { ASSERT_EQ(new_cf_opt.arena_block_size, 22U); ASSERT_EQ(new_cf_opt.disable_auto_compactions, true); ASSERT_EQ(new_cf_opt.compaction_style, kCompactionStyleLevel); - ASSERT_EQ(new_cf_opt.verify_checksums_in_compaction, false); ASSERT_EQ(new_cf_opt.compaction_options_fifo.max_table_files_size, static_cast(23)); ASSERT_EQ(new_cf_opt.max_sequential_skip_in_iterations, @@ -185,7 +184,6 @@ TEST_F(OptionsTest, GetOptionsFromMapTest) { ASSERT_EQ(new_cf_opt.memtable_huge_page_size, 28U); ASSERT_EQ(new_cf_opt.bloom_locality, 29U); ASSERT_EQ(new_cf_opt.max_successive_merges, 30U); - ASSERT_EQ(new_cf_opt.min_partial_merge_operands, 31U); ASSERT_TRUE(new_cf_opt.prefix_extractor != nullptr); ASSERT_EQ(new_cf_opt.optimize_filters_for_hits, true); ASSERT_EQ(std::string(new_cf_opt.prefix_extractor->Name()), diff --git a/util/testutil.cc b/util/testutil.cc index 02de9a239..7b432ab59 100644 --- a/util/testutil.cc +++ b/util/testutil.cc @@ -303,7 +303,6 @@ void RandomInitCFOptions(ColumnFamilyOptions* cf_opt, Random* rnd) { cf_opt->optimize_filters_for_hits = rnd->Uniform(2); cf_opt->paranoid_file_checks = rnd->Uniform(2); cf_opt->purge_redundant_kvs_while_flush = rnd->Uniform(2); - cf_opt->verify_checksums_in_compaction = rnd->Uniform(2); cf_opt->force_consistency_checks = rnd->Uniform(2); // double options @@ -339,7 +338,6 @@ void RandomInitCFOptions(ColumnFamilyOptions* cf_opt, Random* rnd) { // uint32_t options cf_opt->bloom_locality = rnd->Uniform(10000); - cf_opt->min_partial_merge_operands = rnd->Uniform(10000); cf_opt->max_bytes_for_level_base = rnd->Uniform(10000); // uint64_t options