Skip to content

Commit cf9cd5b

Browse files
authored
Revert "Snapshot release triggered compaction without multiple tombstones" (#396)
Signed-off-by: Yang Zhang <[email protected]>
1 parent 31eb76b commit cf9cd5b

File tree

4 files changed

+5
-31
lines changed

4 files changed

+5
-31
lines changed

HISTORY.md

+1-2
Original file line numberDiff line numberDiff line change
@@ -1030,8 +1030,6 @@ Note: The next release will be major release 7.0. See https://github.com/faceboo
10301030
* BackupEngineOptions::sync (default true) now applies to restoring backups in addition to creating backups. This could slow down restores, but ensures they are fully persisted before returning OK. (Consider increasing max_background_operations to improve performance.)
10311031

10321032
## 6.23.0 (2021-07-16)
1033-
### Behavior Changes
1034-
* Obsolete keys in the bottommost level that were preserved for a snapshot will now be cleaned upon snapshot release in all cases. This form of compaction (snapshot release triggered compaction) previously had an artificial limitation that multiple tombstones needed to be present.
10351033
### Bug Fixes
10361034
* Blob file checksums are now printed in hexadecimal format when using the `manifest_dump` `ldb` command.
10371035
* `GetLiveFilesMetaData()` now populates the `temperature`, `oldest_ancester_time`, and `file_creation_time` fields of its `LiveFileMetaData` results when the information is available. Previously these fields always contained zero indicating unknown.
@@ -1056,6 +1054,7 @@ Note: The next release will be major release 7.0. See https://github.com/faceboo
10561054
### Behavior Changes
10571055
* Added two additional tickers, MEMTABLE_PAYLOAD_BYTES_AT_FLUSH and MEMTABLE_GARBAGE_BYTES_AT_FLUSH. These stats can be used to estimate the ratio of "garbage" (outdated) bytes in the memtable that are discarded at flush time.
10581056
* Added API comments clarifying safe usage of Disable/EnableManualCompaction and EventListener callbacks for compaction.
1057+
10591058
### Bug Fixes
10601059
* fs_posix.cc GetFreeSpace() always report disk space available to root even when running as non-root. Linux defaults often have disk mounts with 5 to 10 percent of total space reserved only for root. Out of space could result for non-root users.
10611060
* Subcompactions are now disabled when user-defined timestamps are used, since the subcompaction boundary picking logic is currently not timestamp-aware, which could lead to incorrect results when different subcompactions process keys that only differ by timestamp.

db/version_set.cc

+2-1
Original file line numberDiff line numberDiff line change
@@ -4216,7 +4216,8 @@ void VersionStorageInfo::ComputeBottommostFilesMarkedForCompaction(
42164216

42174217
for (auto& level_and_file : bottommost_files_) {
42184218
if (!level_and_file.second->being_compacted &&
4219-
level_and_file.second->fd.largest_seqno != 0) {
4219+
level_and_file.second->fd.largest_seqno != 0 &&
4220+
level_and_file.second->num_deletions > 1) {
42204221
// largest_seqno might be nonzero due to containing the final key in an
42214222
// earlier compaction, whose seqnum we didn't zero out.
42224223
if (level_and_file.second->fd.largest_seqno < oldest_snapshot_seqnum_) {

utilities/blob_db/blob_db_test.cc

+2-10
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,7 @@ TEST_F(BlobDBTest, EnableDisableCompressionGC) {
570570
Random rnd(301);
571571
BlobDBOptions bdb_options;
572572
bdb_options.min_blob_size = 0;
573+
bdb_options.enable_garbage_collection = true;
573574
bdb_options.garbage_collection_cutoff = 1.0;
574575
bdb_options.disable_background_tasks = true;
575576
bdb_options.compression = kSnappyCompression;
@@ -598,11 +599,6 @@ TEST_F(BlobDBTest, EnableDisableCompressionGC) {
598599
ASSERT_EQ(2, blob_files.size());
599600
ASSERT_EQ(kNoCompression, blob_files[1]->GetCompressionType());
600601

601-
// Enable GC. If we do it earlier the snapshot release triggered compaction
602-
// may compact files and trigger GC before we can verify there are two files.
603-
bdb_options.enable_garbage_collection = true;
604-
Reopen(bdb_options);
605-
606602
// Trigger compaction
607603
ASSERT_OK(blob_db_->CompactRange(CompactRangeOptions(), nullptr, nullptr));
608604
blob_db_impl()->TEST_DeleteObsoleteFiles();
@@ -641,6 +637,7 @@ TEST_F(BlobDBTest, ChangeCompressionGC) {
641637
Random rnd(301);
642638
BlobDBOptions bdb_options;
643639
bdb_options.min_blob_size = 0;
640+
bdb_options.enable_garbage_collection = true;
644641
bdb_options.garbage_collection_cutoff = 1.0;
645642
bdb_options.disable_background_tasks = true;
646643
bdb_options.compression = kLZ4Compression;
@@ -670,11 +667,6 @@ TEST_F(BlobDBTest, ChangeCompressionGC) {
670667
ASSERT_EQ(2, blob_files.size());
671668
ASSERT_EQ(kSnappyCompression, blob_files[1]->GetCompressionType());
672669

673-
// Enable GC. If we do it earlier the snapshot release triggered compaction
674-
// may compact files and trigger GC before we can verify there are two files.
675-
bdb_options.enable_garbage_collection = true;
676-
Reopen(bdb_options);
677-
678670
ASSERT_OK(blob_db_->CompactRange(CompactRangeOptions(), nullptr, nullptr));
679671
VerifyDB(data);
680672

utilities/transactions/write_prepared_transaction_test.cc

-18
Original file line numberDiff line numberDiff line change
@@ -2582,7 +2582,6 @@ TEST_P(WritePreparedTransactionTest, ReleaseSnapshotDuringCompaction) {
25822582
const size_t snapshot_cache_bits = 7; // same as default
25832583
const size_t commit_cache_bits = 0; // minimum commit cache
25842584
UpdateTransactionDBOptions(snapshot_cache_bits, commit_cache_bits);
2585-
options.disable_auto_compactions = true;
25862585
ASSERT_OK(ReOpen());
25872586

25882587
ASSERT_OK(db->Put(WriteOptions(), "key1", "value1_1"));
@@ -2602,13 +2601,7 @@ TEST_P(WritePreparedTransactionTest, ReleaseSnapshotDuringCompaction) {
26022601
VerifyKeys({{"key1", "value1_1"}}, snapshot2);
26032602
// Add a flush to avoid compaction to fallback to trivial move.
26042603

2605-
// The callback might be called twice, record the calling state to
2606-
// prevent double calling.
2607-
bool callback_finished = false;
26082604
auto callback = [&](void*) {
2609-
if (callback_finished) {
2610-
return;
2611-
}
26122605
// Release snapshot1 after CompactionIterator init.
26132606
// CompactionIterator need to figure out the earliest snapshot
26142607
// that can see key1:value1_2 is kMaxSequenceNumber, not
@@ -2617,7 +2610,6 @@ TEST_P(WritePreparedTransactionTest, ReleaseSnapshotDuringCompaction) {
26172610
// Add some keys to advance max_evicted_seq.
26182611
ASSERT_OK(db->Put(WriteOptions(), "key3", "value3"));
26192612
ASSERT_OK(db->Put(WriteOptions(), "key4", "value4"));
2620-
callback_finished = true;
26212613
};
26222614
SyncPoint::GetInstance()->SetCallBack("CompactionIterator:AfterInit",
26232615
callback);
@@ -2639,7 +2631,6 @@ TEST_P(WritePreparedTransactionTest, ReleaseSnapshotDuringCompaction2) {
26392631
const size_t snapshot_cache_bits = 7; // same as default
26402632
const size_t commit_cache_bits = 0; // minimum commit cache
26412633
UpdateTransactionDBOptions(snapshot_cache_bits, commit_cache_bits);
2642-
options.disable_auto_compactions = true;
26432634
ASSERT_OK(ReOpen());
26442635

26452636
ASSERT_OK(db->Put(WriteOptions(), "key1", "value1"));
@@ -2690,7 +2681,6 @@ TEST_P(WritePreparedTransactionTest, ReleaseSnapshotDuringCompaction3) {
26902681
const size_t snapshot_cache_bits = 7; // same as default
26912682
const size_t commit_cache_bits = 1; // commit cache size = 2
26922683
UpdateTransactionDBOptions(snapshot_cache_bits, commit_cache_bits);
2693-
options.disable_auto_compactions = true;
26942684
ASSERT_OK(ReOpen());
26952685

26962686
// Add a dummy key to evict v2 commit cache, but keep v1 commit cache.
@@ -2720,18 +2710,11 @@ TEST_P(WritePreparedTransactionTest, ReleaseSnapshotDuringCompaction3) {
27202710
add_dummy();
27212711
auto* s2 = db->GetSnapshot();
27222712

2723-
// The callback might be called twice, record the calling state to
2724-
// prevent double calling.
2725-
bool callback_finished = false;
27262713
auto callback = [&](void*) {
2727-
if (callback_finished) {
2728-
return;
2729-
}
27302714
db->ReleaseSnapshot(s1);
27312715
// Add some dummy entries to trigger s1 being cleanup from old_commit_map.
27322716
add_dummy();
27332717
add_dummy();
2734-
callback_finished = true;
27352718
};
27362719
SyncPoint::GetInstance()->SetCallBack("CompactionIterator:AfterInit",
27372720
callback);
@@ -2749,7 +2732,6 @@ TEST_P(WritePreparedTransactionTest, ReleaseEarliestSnapshotDuringCompaction) {
27492732
const size_t snapshot_cache_bits = 7; // same as default
27502733
const size_t commit_cache_bits = 0; // minimum commit cache
27512734
UpdateTransactionDBOptions(snapshot_cache_bits, commit_cache_bits);
2752-
options.disable_auto_compactions = true;
27532735
ASSERT_OK(ReOpen());
27542736

27552737
ASSERT_OK(db->Put(WriteOptions(), "key1", "value1"));

0 commit comments

Comments
 (0)