Allow manual compactions to run in parallel by default (#10317)

Summary:
This PR changes the default value of
`CompactRangeOptions::exclusive_manual_compaction` from true to false so
manual `CompactRange()`s can run in parallel with other compactions. I
believe no artificial parallelism restriction is the intuitive behavior
so feel the old default value is a trap, which I have fallen into
several times, including yesterday.
`CompactRangeOptions::exclusive_manual_compaction == false` has been
used in both our correctness test and in production for years so should
be reasonably safe.

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

Reviewed By: jay-zhuang

Differential Revision: D37659392

Pulled By: ajkr

fbshipit-source-id: 504915e978bbe300b79483d064070c75e93d91e5
main
Andrew Kryczka 2 years ago committed by Facebook GitHub Bot
parent 87649d3288
commit c7ccbb33a6
  1. 1
      HISTORY.md
  2. 5
      db/db_compaction_test.cc
  3. 3
      db/db_impl/db_impl_compaction_flush.cc
  4. 7
      include/rocksdb/options.h

@ -10,6 +10,7 @@
### Public API changes
* Removed Customizable support for RateLimiter and removed its CreateFromString() and Type() functions.
* `CompactRangeOptions::exclusive_manual_compaction` is now false by default. This ensures RocksDB does not introduce artificial parallelism limitations by default.
### Bug Fixes
* Fix a bug where `GenericRateLimiter` could revert the bandwidth set dynamically using `SetBytesPerSecond()` when a user configures a structure enclosing it, e.g., using `GetOptionsFromString()` to configure an `Options` that references an existing `RateLimiter` object.

@ -2660,8 +2660,9 @@ TEST_F(DBCompactionTest, ManualAutoRace) {
TEST_SYNC_POINT("DBCompactionTest::ManualAutoRace:1");
// The auto compaction will wait until the manual compaction is registerd
// before processing so that it will be cancelled.
ASSERT_OK(dbfull()->CompactRange(CompactRangeOptions(), handles_[1], nullptr,
nullptr));
CompactRangeOptions cro;
cro.exclusive_manual_compaction = true;
ASSERT_OK(dbfull()->CompactRange(cro, handles_[1], nullptr, nullptr));
ASSERT_EQ("0,1", FilesPerLevel(1));
// Eventually the cancelled compaction will be rescheduled and executed.

@ -1853,8 +1853,7 @@ Status DBImpl::RunManualCompaction(
// jobs drops to zero. This used to be needed to ensure that this manual
// compaction can compact any range of keys/files. Now it is optional
// (see `CompactRangeOptions::exclusive_manual_compaction`). The use case for
// `exclusive_manual_compaction=true` (the default) is unclear beyond not
// trusting the new code.
// `exclusive_manual_compaction=true` is unclear beyond not trusting the code.
//
// HasPendingManualCompaction() is true when at least one thread is inside
// RunManualCompaction(), i.e. during that time no other compaction will

@ -1843,8 +1843,11 @@ enum class BlobGarbageCollectionPolicy {
// CompactRangeOptions is used by CompactRange() call.
struct CompactRangeOptions {
// If true, no other compaction will run at the same time as this
// manual compaction
bool exclusive_manual_compaction = true;
// manual compaction.
//
// Default: false
bool exclusive_manual_compaction = false;
// If true, compacted files will be moved to the minimum level capable
// of holding the data or given level (specified non-negative target_level).
bool change_level = false;

Loading…
Cancel
Save