From: Maysam Yabandeh Date: Tue, 20 Aug 2019 18:38:15 +0000 (-0700) Subject: Disable snapshot refresh feature when snap_refresh_nanos is 0 (#5724) X-Git-Tag: v6.2.4~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=dd757ffde0117af50ea22c295469389f84501f11;p=rocksdb.git Disable snapshot refresh feature when snap_refresh_nanos is 0 (#5724) Summary: The comments of snap_refresh_nanos advertise that the snapshot refresh feature will be disabled when the option is set to 0. This contract is however not honored in the code: https://github.com/facebook/rocksdb/pull/5278 The patch fixes that and also adds an assert to ensure that the feature is not used when the option is zero. Pull Request resolved: https://github.com/facebook/rocksdb/pull/5724 Differential Revision: D16918185 Pulled By: maysamyabandeh fbshipit-source-id: fec167287df7d85093e087fc39c0eb243e3bbd7e --- diff --git a/HISTORY.md b/HISTORY.md index d83bde76..06c2bd0e 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,4 +1,8 @@ # Rocksdb Change Log +## 6.2.4 (9/18/2019) +### Bug Fixes +* Disable snap_refresh_nanos by default. The feature is to be deprecated in the next release. + ## 6.2.3 (9/3/2019) ### Bug Fixes * Fix a bug in file ingestion caused by incorrect file number allocation when the number of column families involved in the ingestion exceeds 2. diff --git a/db/compaction_iterator.h b/db/compaction_iterator.h index 6ab43b1b..df25eaa9 100644 --- a/db/compaction_iterator.h +++ b/db/compaction_iterator.h @@ -39,6 +39,7 @@ class SnapshotListFetchCallback { virtual void Refresh(std::vector* snapshots, SequenceNumber max) = 0; inline bool TimeToRefresh(const size_t key_index) { + assert(snap_refresh_nanos_ != 0); // skip the key if key_index % every_nth_key (which is of power 2) is not 0. if ((key_index & every_nth_key_minus_one_) != 0) { return false; diff --git a/db/compaction_job_test.cc b/db/compaction_job_test.cc index 60394cc9..49f7384f 100644 --- a/db/compaction_job_test.cc +++ b/db/compaction_job_test.cc @@ -966,7 +966,7 @@ TEST_F(CompactionJobTest, SnapshotRefresh) { public: SnapshotListFetchCallbackTest(Env* env, Random64& rand, std::vector* snapshots) - : SnapshotListFetchCallback(env, 0 /*no time delay*/, + : SnapshotListFetchCallback(env, 1 /*short time delay*/, 1 /*fetch after each key*/), rand_(rand), snapshots_(snapshots) {} diff --git a/db/db_impl_compaction_flush.cc b/db/db_impl_compaction_flush.cc index 3fbf24e4..865acf5c 100644 --- a/db/db_impl_compaction_flush.cc +++ b/db/db_impl_compaction_flush.cc @@ -1007,8 +1007,10 @@ Status DBImpl::CompactFilesImpl( c->mutable_cf_options()->paranoid_file_checks, c->mutable_cf_options()->report_bg_io_stats, dbname_, &compaction_job_stats, Env::Priority::USER, - immutable_db_options_.max_subcompactions <= 1 ? &fetch_callback - : nullptr); + immutable_db_options_.max_subcompactions <= 1 && + c->mutable_cf_options()->snap_refresh_nanos > 0 + ? &fetch_callback + : nullptr); // Creating a compaction influences the compaction score because the score // takes running compactions into account (by skipping files that are already @@ -2669,8 +2671,10 @@ Status DBImpl::BackgroundCompaction(bool* made_progress, &event_logger_, c->mutable_cf_options()->paranoid_file_checks, c->mutable_cf_options()->report_bg_io_stats, dbname_, &compaction_job_stats, thread_pri, - immutable_db_options_.max_subcompactions <= 1 ? &fetch_callback - : nullptr); + immutable_db_options_.max_subcompactions <= 1 && + c->mutable_cf_options()->snap_refresh_nanos > 0 + ? &fetch_callback + : nullptr); compaction_job.Prepare(); NotifyOnCompactionBegin(c->column_family_data(), c.get(), status,