Fix assertion failure in ConstructFragmentedRangeTombstones() (#12796)

Summary:
the assertion `assert(!IsFragmentedRangeTombstonesConstructed(false));` assumes ConstructFragmentedRangeTombstones() is called only once for a memtable. This is not true since SwitchMemtable() can be called multiple times on the same live memtable, if a previous attempt fails. So remove the assertion in this PR and simplify relevant code.

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

Test Plan: the exact condition to trigger manifest write in SwitchMemtable() is complicated. Will monitor crash test to see if there's no more failure.

Reviewed By: hx235

Differential Revision: D58913310

Pulled By: cbi42

fbshipit-source-id: 458bb9eebcf6743e9001186fcb757e4b50e8a5d2
This commit is contained in:
Changyu Bi 2024-06-22 11:31:16 -07:00 committed by Facebook GitHub Bot
parent 981fd432fa
commit b4a84efb4e
2 changed files with 13 additions and 12 deletions

View File

@ -618,8 +618,9 @@ FragmentedRangeTombstoneIterator* MemTable::NewRangeTombstoneIteratorInternal(
}
void MemTable::ConstructFragmentedRangeTombstones() {
assert(!IsFragmentedRangeTombstonesConstructed(false));
// There should be no concurrent Construction
// There should be no concurrent Construction.
// We could also check fragmented_range_tombstone_list_ to avoid repeate
// constructions. We just construct them here again to be safe.
if (!is_range_del_table_empty_.load(std::memory_order_relaxed)) {
// TODO: plumb Env::IOActivity, Env::IOPriority
auto* unfragmented_iter = new MemTableIterator(

View File

@ -534,21 +534,21 @@ class MemTable {
// Returns a heuristic flush decision
bool ShouldFlushNow();
// Updates `fragmented_range_tombstone_list_` that will be used to serve reads
// when this memtable becomes an immutable memtable (in some
// MemtableListVersion::memlist_). Should be called when this memtable is
// about to become immutable. May be called multiple times since
// SwitchMemtable() may fail.
void ConstructFragmentedRangeTombstones();
// Returns whether a fragmented range tombstone list is already constructed
// for this memtable. It should be constructed right before a memtable is
// added to an immutable memtable list. Note that if a memtable does not have
// any range tombstone, then no range tombstone list will ever be constructed.
// @param allow_empty Specifies whether a memtable with no range tombstone is
// considered to have its fragmented range tombstone list constructed.
bool IsFragmentedRangeTombstonesConstructed(bool allow_empty = true) const {
if (allow_empty) {
return fragmented_range_tombstone_list_.get() != nullptr ||
is_range_del_table_empty_;
} else {
return fragmented_range_tombstone_list_.get() != nullptr;
}
// any range tombstone, then no range tombstone list will ever be constructed
// and true is returned in that case.
bool IsFragmentedRangeTombstonesConstructed() const {
return fragmented_range_tombstone_list_.get() != nullptr ||
is_range_del_table_empty_;
}
// Get the newest user-defined timestamp contained in this MemTable. Check