Summary: **Context:** Adding assertion `!PendingPut()&&!PendingDelete()` in `ExpectedValute::Exists()` surfaced a couple improper usages of `ExpectedValute::Exists()` in the crash test - Commit phase of `ExpectedValue::Delete()`/`SyncDelete()`: When we issue delete to expected value during commit phase or `SyncDelete()` (used in crash recovery verification) as below, we don't really care about the result.d458331ee9/db_stress_tool/expected_state.cc (L73)
d458331ee9/db_stress_tool/expected_value.cc (L52)
That means, we don't really need to check for `Exists()`d458331ee9/db_stress_tool/expected_value.cc (L24-L26)
. This actually gives an alternative solution tob65e29a4a9
to solve false-positive assertion violation. - TestMultiGetXX() path: `Exists()` is called without holding the lock as requiredf63428bcc7/db_stress_tool/no_batched_ops_stress.cc (L2688)
``` void MaybeAddKeyToTxnForRYW( ThreadState* thread, int column_family, int64_t key, Transaction* txn, std::unordered_map<std::string, ExpectedValue>& ryw_expected_values) { assert(thread); assert(txn); SharedState* const shared = thread->shared; assert(shared); if (!shared->AllowsOverwrite(key) && shared->Exists(column_family, key)) { // Just do read your write checks for keys that allow overwrites. return; } // With a 1 in 10 probability, insert the just added key in the batch // into the transaction. This will create an overlap with the MultiGet // keys and exercise some corner cases in the code if (thread->rand.OneIn(10)) { ```f63428bcc7/db_stress_tool/expected_state.h (L74-L76)
The assertion also failed if db stress compaction filter was invoked before crash recovery verification (`VerifyDB()`->`VerifyOrSyncValue()`) finishes.f63428bcc7/db_stress_tool/db_stress_compaction_filter.h (L53)
It failed because it can encounter a key with pending state when checking for `Exists()` since that key's expected state has not been sync-ed with db state in `VerifyOrSyncValue()`.f63428bcc7/db_stress_tool/no_batched_ops_stress.cc (L2579-L2591)
**Summary:** This PR fixes above issues by - not checking `Exists()` in commit phase/`SyncDelete()` - using the concurrent version of key existence check like in other read - conditionally temporarily disabling compaction till after crash recovery verification succeeds() And add back the assertion `!PendingPut()&&!PendingDelete()` Pull Request resolved: https://github.com/facebook/rocksdb/pull/12933 Test Plan: Rehearsal CI Reviewed By: cbi42 Differential Revision: D61214889 Pulled By: hx235 fbshipit-source-id: ef25ba896e64330ddf330182314981516880c3e4
122 lines
3.9 KiB
C++
122 lines
3.9 KiB
C++
// Copyright (c) 2021-present, Facebook, Inc. All rights reserved.
|
|
// This source code is licensed under both the GPLv2 (found in the
|
|
// COPYING file in the root directory) and Apache 2.0 License
|
|
// (found in the LICENSE.Apache file in the root directory).
|
|
|
|
#ifdef GFLAGS
|
|
|
|
#include "db_stress_tool/expected_value.h"
|
|
|
|
#include <atomic>
|
|
|
|
namespace ROCKSDB_NAMESPACE {
|
|
void ExpectedValue::Put(bool pending) {
|
|
if (pending) {
|
|
SetPendingWrite();
|
|
} else {
|
|
SetValueBase(NextValueBase());
|
|
ClearDeleted();
|
|
ClearPendingWrite();
|
|
}
|
|
}
|
|
|
|
bool ExpectedValue::Delete(bool pending) {
|
|
if (pending && !Exists()) {
|
|
return false;
|
|
}
|
|
if (pending) {
|
|
SetPendingDel();
|
|
} else {
|
|
SetDelCounter(NextDelCounter());
|
|
SetDeleted();
|
|
ClearPendingDel();
|
|
}
|
|
return true;
|
|
}
|
|
|
|
void ExpectedValue::SyncPut(uint32_t value_base) {
|
|
assert(ExpectedValue::IsValueBaseValid(value_base));
|
|
|
|
SetValueBase(value_base);
|
|
ClearDeleted();
|
|
ClearPendingWrite();
|
|
|
|
// This is needed in case crash happens during a pending delete of the key
|
|
// assocated with this expected value
|
|
ClearPendingDel();
|
|
}
|
|
|
|
void ExpectedValue::SyncPendingPut() { Put(true /* pending */); }
|
|
|
|
void ExpectedValue::SyncDelete() {
|
|
Delete(false /* pending */);
|
|
// This is needed in case crash happens during a pending write of the key
|
|
// assocated with this expected value
|
|
ClearPendingWrite();
|
|
}
|
|
|
|
uint32_t ExpectedValue::GetFinalValueBase() const {
|
|
return PendingWrite() ? NextValueBase() : GetValueBase();
|
|
}
|
|
|
|
uint32_t ExpectedValue::GetFinalDelCounter() const {
|
|
return PendingDelete() ? NextDelCounter() : GetDelCounter();
|
|
}
|
|
|
|
bool ExpectedValueHelper::MustHaveNotExisted(
|
|
ExpectedValue pre_read_expected_value,
|
|
ExpectedValue post_read_expected_value) {
|
|
const bool pre_read_expected_deleted = pre_read_expected_value.IsDeleted();
|
|
|
|
const uint32_t pre_read_expected_value_base =
|
|
pre_read_expected_value.GetValueBase();
|
|
|
|
const uint32_t post_read_expected_final_value_base =
|
|
post_read_expected_value.GetFinalValueBase();
|
|
|
|
const bool during_read_no_write_happened =
|
|
(pre_read_expected_value_base == post_read_expected_final_value_base);
|
|
return pre_read_expected_deleted && during_read_no_write_happened;
|
|
}
|
|
|
|
bool ExpectedValueHelper::MustHaveExisted(
|
|
ExpectedValue pre_read_expected_value,
|
|
ExpectedValue post_read_expected_value) {
|
|
const bool pre_read_expected_not_deleted =
|
|
!pre_read_expected_value.IsDeleted();
|
|
|
|
const uint32_t pre_read_expected_del_counter =
|
|
pre_read_expected_value.GetDelCounter();
|
|
const uint32_t post_read_expected_final_del_counter =
|
|
post_read_expected_value.GetFinalDelCounter();
|
|
|
|
const bool during_read_no_delete_happened =
|
|
(pre_read_expected_del_counter == post_read_expected_final_del_counter);
|
|
|
|
return pre_read_expected_not_deleted && during_read_no_delete_happened;
|
|
}
|
|
|
|
bool ExpectedValueHelper::InExpectedValueBaseRange(
|
|
uint32_t value_base, ExpectedValue pre_read_expected_value,
|
|
ExpectedValue post_read_expected_value) {
|
|
assert(ExpectedValue::IsValueBaseValid(value_base));
|
|
|
|
const uint32_t pre_read_expected_value_base =
|
|
pre_read_expected_value.GetValueBase();
|
|
const uint32_t post_read_expected_final_value_base =
|
|
post_read_expected_value.GetFinalValueBase();
|
|
|
|
if (pre_read_expected_value_base <= post_read_expected_final_value_base) {
|
|
const uint32_t lower_bound = pre_read_expected_value_base;
|
|
const uint32_t upper_bound = post_read_expected_final_value_base;
|
|
return lower_bound <= value_base && value_base <= upper_bound;
|
|
} else {
|
|
const uint32_t upper_bound_1 = post_read_expected_final_value_base;
|
|
const uint32_t lower_bound_2 = pre_read_expected_value_base;
|
|
const uint32_t upper_bound_2 = ExpectedValue::GetValueBaseMask();
|
|
return (value_base <= upper_bound_1) ||
|
|
(lower_bound_2 <= value_base && value_base <= upper_bound_2);
|
|
}
|
|
}
|
|
} // namespace ROCKSDB_NAMESPACE
|
|
#endif // GFLAGS
|