[BUG] bottommost level garbage key versions leak forever after compaction
- Dominant language
- C++
- Stars
- 32.1k
- Forks
- 6.9k
- Avg merge
- 32m
- Merged PRs (30d)
- 1
Description
> Note: Please use Issues only for bug reports. For questions, discussions, feature requests, etc. post to dev group: https://groups.google.com/forum/#!forum/rocksdb or https://www.facebook.com/groups/rocksdb.dev
### Expected behavior
Timestamped keys in bottommost level pined by full_history_ts_low should have a chance to be compacted when full_history_ts_low moves forward
### Actual behavior
Follow the steps below, you will find the keys in bottommost level pined by full_history_ts_low leak
### Steps to reproduce the behavior
Step1: set full_history_low to 0 to pin all history
Step2: put some keys with timestamp=1, and then delete them all with timestap=1
Step3: Flush the keys to l0 and CompactRange to ensure the keys got into bottommost level
Step4: assert there are some files in bottommost files, the files all have largest_seq = 0, so the compacted output files will never got another chance to be garbage-collected.
Explainations to Step4:
as implemented below
https://github.com/facebook/rocksdb/blob/27cf09172c4f212d0aef23b23c802bb791ec9209/db/version_set.cc#L3988-L4000
files in bottommost files can be compacted iff fd.largest_seqno > 0
### Suggestion to fix this bug
Add minTs and maxTs to file's meta data, each time full_history_low moves forward, re-caculate files which need compaction.
### example code
```
TEST_F(TimestampCompatibleCompactionTest, BottommostLevelCompactionTest) {
Options options = CurrentOptions();
options.env = env_;
options.sst_partitioner_factory =
std::make_shared();
options.comparator = test::BytewiseComparatorWithU64TsWrapper();
constexpr int kNumFiles = 4;
options.level0_file_num_compaction_trigger = kNumFiles;
DestroyAndReopen(options);
uint64_t full_history_low = 0;
ASSERT_OK(db_->IncreaseFullHistoryTsLow(db_->DefaultColumnFamily(), Timestamp(full_history_low)));
constexpr int kKeysPerFile = 2;
const std::string user_key = "foo";
constexpr uint64_t start_ts = 1;
uint64_t cur_ts = start_ts;
// Generate some initial files in both L0 and L1.
for (int k = 0; k < kNumFiles; ++k) {
for (int i = 0; i < kKeysPerFile; ++i) {
ASSERT_OK(db_->Put(WriteOptions(),
user_key + std::to_string(i),
Timestamp(cur_ts),
"v"));
++cur_ts;
}
ASSERT_OK(db_->Flush(FlushOptions()));
}
for (int k = 0; k < kNumFiles; ++k) {
for (int i = 0; i < kKeysPerFile; ++i) {
ASSERT_OK(db_->Delete(WriteOptions(),
user_key + std::to_string(i),
Timestamp(cur_ts)));
++cur_ts;
}
ASSERT_OK(db_->Flush(FlushOptions()));
}
ASSERT_OK(dbfull()->TEST_WaitForCompact());
ASSERT_EQ(2 * kNumFiles * kKeysPerFile,
NumTableFilesAtLevel(/*level=*/1, /*cf=*/0));
std::vector file_metadatas;
db_->GetLiveFilesMetaData(&file_metadatas);
for (auto const& file : file_metadatas) {
ASSERT_EQ(0, file.largest_seqno);
}
}
```
Contributor guide
Research direction
Start with the provided TimestampCompatibleCompactionTest example and reproduce it in the timestamp-compatible compaction tests. Read the referenced bottommost-file logic in db/version_set.cc around lines 3988-4000, then trace how full_history_ts_low changes are handled. Done means bottommost files whose history is no longer pinned can be selected for compaction and the regression test passes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100