andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Disable snapshot refresh feature when snap_refresh_nanos is 0 (#5724)

Browse Source 
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
This commit is contained in:
Maysam Yabandeh 2019-08-20 11:38:15 -07:00 committed by Facebook Github Bot
parent 3552473668
commit 7bc18e2727
4 changed files with 11 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
HISTORY.md
View File

@ -2,6 +2,7 @@
## Unreleased ## Unreleased
### Bug Fixes ### Bug Fixes
* Fixed a number of data races in BlobDB. * Fixed a number of data races in BlobDB.
* Fix a bug where the compaction snapshot refresh feature is not disabled as advertised when `snap_refresh_nanos` is set to 0..
### New Features ### New Features
* VerifyChecksum() by default will issue readahead. Allow ReadOptions to be passed in to those functions to override the readhead size. For checksum verifying before external SST file ingestion, a new option IngestExternalFileOptions.verify_checksums_readahead_size, is added for this readahead setting. * VerifyChecksum() by default will issue readahead. Allow ReadOptions to be passed in to those functions to override the readhead size. For checksum verifying before external SST file ingestion, a new option IngestExternalFileOptions.verify_checksums_readahead_size, is added for this readahead setting.

1
db/compaction/compaction_iterator.h
View File

@ -39,6 +39,7 @@ class SnapshotListFetchCallback {
virtual void Refresh(std::vector<SequenceNumber>* snapshots, virtual void Refresh(std::vector<SequenceNumber>* snapshots,
SequenceNumber max) = 0; SequenceNumber max) = 0;
inline bool TimeToRefresh(const size_t key_index) { 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. // 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) { if ((key_index & every_nth_key_minus_one_) != 0) {
return false; return false;

2
db/compaction/compaction_job_test.cc
View File

@ -964,7 +964,7 @@ TEST_F(CompactionJobTest, SnapshotRefresh) {
public: public:
SnapshotListFetchCallbackTest(Env* env, Random64& rand, SnapshotListFetchCallbackTest(Env* env, Random64& rand,
std::vector<SequenceNumber>* snapshots) std::vector<SequenceNumber>* snapshots)
: SnapshotListFetchCallback(env, 0 /*no time delay*/, : SnapshotListFetchCallback(env, 1 /*short time delay*/,
1 /*fetch after each key*/), 1 /*fetch after each key*/),
rand_(rand), rand_(rand),
snapshots_(snapshots) {} snapshots_(snapshots) {}

12
db/db_impl/db_impl_compaction_flush.cc
View File

@ -1008,8 +1008,10 @@ Status DBImpl::CompactFilesImpl(
c->mutable_cf_options()->paranoid_file_checks, c->mutable_cf_options()->paranoid_file_checks,
c->mutable_cf_options()->report_bg_io_stats, dbname_, c->mutable_cf_options()->report_bg_io_stats, dbname_,
&compaction_job_stats, Env::Priority::USER, &compaction_job_stats, Env::Priority::USER,
immutable_db_options_.max_subcompactions <= 1 ? &fetch_callback immutable_db_options_.max_subcompactions <= 1 &&
: nullptr); c->mutable_cf_options()->snap_refresh_nanos > 0
? &fetch_callback
: nullptr);
// Creating a compaction influences the compaction score because the score // Creating a compaction influences the compaction score because the score
// takes running compactions into account (by skipping files that are already // takes running compactions into account (by skipping files that are already
@ -2737,8 +2739,10 @@ Status DBImpl::BackgroundCompaction(bool* made_progress,
&event_logger_, c->mutable_cf_options()->paranoid_file_checks, &event_logger_, c->mutable_cf_options()->paranoid_file_checks,
c->mutable_cf_options()->report_bg_io_stats, dbname_, c->mutable_cf_options()->report_bg_io_stats, dbname_,
&compaction_job_stats, thread_pri, &compaction_job_stats, thread_pri,
immutable_db_options_.max_subcompactions <= 1 ? &fetch_callback immutable_db_options_.max_subcompactions <= 1 &&
: nullptr); c->mutable_cf_options()->snap_refresh_nanos > 0
? &fetch_callback
: nullptr);
compaction_job.Prepare(); compaction_job.Prepare();
NotifyOnCompactionBegin(c->column_family_data(), c.get(), status, NotifyOnCompactionBegin(c->column_family_data(), c.get(), status,