wbtiger
/
foundationdb
mirror of https://github.com/apple/foundationdb.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add test coverage for SpecialKeyRangeAsyncImpl::getRange (#7671) 

* Add getRange test coverage for SpecialKeyRangeAsyncImpl

* Fix the bug in SpecialKeyRangeAsyncImpl found by the test

* Refactor ConflictingKeysImpl::getRange to use containedRanges to simplify the code

* Fix file format

* Initialize SpecialKeyRangeAsyncImpl cache with correct end key

* Add release notes

* Revert "Refactor ConflictingKeysImpl::getRange to use containedRanges to simplify the code"

This reverts commit fdd298f469.
This commit is contained in:
Chaoguang Lin 2022-08-02 12:04:40 -07:00 committed by GitHub
parent 4b66645d80
commit 48e46cbc81
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 66 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
documentation/sphinx/source/release-notes/release-notes-720.rst
View File

@ -20,6 +20,8 @@ Fixes
-----
* In ``fdbcli``, integer options are now expressed as integers rather than byte strings (e.g. ``option on TIMEOUT 1000``). `(PR #7571) <https://github.com/apple/foundationdb/pull/7571>`_
* Fixed the bug in ``ConflictingKeysImpl::getRange`` which happens when an underlying conflicting range contains the read range. Added additional test coverage for validating random ``getRange`` results from special keys. `(PR #7597) <https://github.com/apple/foundationdb/pull/7597>`_
* Fixed the bug in ``SpecialKeyRangeAsyncImpl::getRange`` that the local cache is updated incorrectly after a cross-module read range if it touched more than one ``SpecialKeyRangeAsyncImpl`` in resolving key selectors. Extended the ``SpecialKeySpaceCorrectness`` workload to catch the bug. `(PR #7671) <https://github.com/apple/foundationdb/pull/7671>`_
Status
------

41
fdbclient/SpecialKeySpace.actor.cpp
View File

@ -148,7 +148,7 @@ RangeResult rywGetRange(ReadYourWritesTransaction* ryw, const KeyRangeRef& kr, c
ACTOR Future<Void> moveKeySelectorOverRangeActor(const SpecialKeyRangeReadImpl* skrImpl,
ReadYourWritesTransaction* ryw,
KeySelector* ks,
Optional<RangeResult>* cache) {
KeyRangeMap<Optional<RangeResult>>* cache) {
// should be removed before calling
ASSERT(!ks->orEqual);
@ -234,7 +234,7 @@ ACTOR Future<Void> normalizeKeySelectorActor(SpecialKeySpace* sks,
KeyRangeRef boundary,
int* actualOffset,
RangeResult* result,
Optional<RangeResult>* cache) {
KeyRangeMap<Optional<RangeResult>>* cache) {
// If offset < 1, where we need to move left, iter points to the range containing at least one smaller key
// (It's a wasting of time to walk through the range whose begin key is same as ks->key)
// (rangeContainingKeyBefore itself handles the case where ks->key == Key())
@ -337,8 +337,9 @@ ACTOR Future<RangeResult> SpecialKeySpace::getRangeAggregationActor(SpecialKeySp
state int actualBeginOffset;
state int actualEndOffset;
state KeyRangeRef moduleBoundary;
// used to cache result from potential first read
state Optional<RangeResult> cache;
// used to cache results from potential first async read
// the current implementation will read the whole range result to save in the cache
state KeyRangeMap<Optional<RangeResult>> cache(Optional<RangeResult>(), specialKeys.end);
if (ryw->specialKeySpaceRelaxed()) {
moduleBoundary = sks->range;
@ -385,7 +386,7 @@ ACTOR Future<RangeResult> SpecialKeySpace::getRangeAggregationActor(SpecialKeySp
KeyRangeRef kr = iter->range();
KeyRef keyStart = kr.contains(begin.getKey()) ? begin.getKey() : kr.begin;
KeyRef keyEnd = kr.contains(end.getKey()) ? end.getKey() : kr.end;
if (iter->value()->isAsync() && cache.present()) {
if (iter->value()->isAsync() && cache.rangeContaining(keyStart).value().present()) {
const SpecialKeyRangeAsyncImpl* ptr = dynamic_cast<const SpecialKeyRangeAsyncImpl*>(iter->value());
RangeResult pairs_ = wait(ptr->getRange(ryw, KeyRangeRef(keyStart, keyEnd), limits, &cache));
pairs = pairs_;
@ -416,7 +417,7 @@ ACTOR Future<RangeResult> SpecialKeySpace::getRangeAggregationActor(SpecialKeySp
KeyRangeRef kr = iter->range();
KeyRef keyStart = kr.contains(begin.getKey()) ? begin.getKey() : kr.begin;
KeyRef keyEnd = kr.contains(end.getKey()) ? end.getKey() : kr.end;
if (iter->value()->isAsync() && cache.present()) {
if (iter->value()->isAsync() && cache.rangeContaining(keyStart).value().present()) {
const SpecialKeyRangeAsyncImpl* ptr = dynamic_cast<const SpecialKeyRangeAsyncImpl*>(iter->value());
RangeResult pairs_ = wait(ptr->getRange(ryw, KeyRangeRef(keyStart, keyEnd), limits, &cache));
pairs = pairs_;
@ -629,12 +630,8 @@ Future<Void> SpecialKeySpace::commit(ReadYourWritesTransaction* ryw) {
return commitActor(this, ryw);
}
SKSCTestImpl::SKSCTestImpl(KeyRangeRef kr) : SpecialKeyRangeRWImpl(kr) {}
Future<RangeResult> SKSCTestImpl::getRange(ReadYourWritesTransaction* ryw,
KeyRangeRef kr,
GetRangeLimits limitsHint) const {
ASSERT(range.contains(kr));
// For SKSCTestRWImpl and SKSCTestAsyncReadImpl
Future<RangeResult> SKSCTestGetRangeBase(ReadYourWritesTransaction* ryw, KeyRangeRef kr, GetRangeLimits limitsHint) {
auto resultFuture = ryw->getRange(kr, CLIENT_KNOBS->TOO_MANY);
// all keys are written to RYW, since GRV is set, the read should happen locally
ASSERT(resultFuture.isReady());
@ -644,11 +641,29 @@ Future<RangeResult> SKSCTestImpl::getRange(ReadYourWritesTransaction* ryw,
return rywGetRange(ryw, kr, kvs);
}
Future<Optional<std::string>> SKSCTestImpl::commit(ReadYourWritesTransaction* ryw) {
SKSCTestRWImpl::SKSCTestRWImpl(KeyRangeRef kr) : SpecialKeyRangeRWImpl(kr) {}
Future<RangeResult> SKSCTestRWImpl::getRange(ReadYourWritesTransaction* ryw,
KeyRangeRef kr,
GetRangeLimits limitsHint) const {
ASSERT(range.contains(kr));
return SKSCTestGetRangeBase(ryw, kr, limitsHint);
}
Future<Optional<std::string>> SKSCTestRWImpl::commit(ReadYourWritesTransaction* ryw) {
ASSERT(false);
return Optional<std::string>();
}
SKSCTestAsyncReadImpl::SKSCTestAsyncReadImpl(KeyRangeRef kr) : SpecialKeyRangeAsyncImpl(kr) {}
Future<RangeResult> SKSCTestAsyncReadImpl::getRange(ReadYourWritesTransaction* ryw,
KeyRangeRef kr,
GetRangeLimits limitsHint) const {
ASSERT(range.contains(kr));
return SKSCTestGetRangeBase(ryw, kr, limitsHint);
}
ReadConflictRangeImpl::ReadConflictRangeImpl(KeyRangeRef kr) : SpecialKeyRangeReadImpl(kr) {}
ACTOR static Future<RangeResult> getReadConflictRangeImpl(ReadYourWritesTransaction* ryw, KeyRange kr) {

23
fdbclient/include/fdbclient/SpecialKeySpace.actor.h
View File

@ -125,7 +125,7 @@ public:
Future<RangeResult> getRange(ReadYourWritesTransaction* ryw,
KeyRangeRef kr,
GetRangeLimits limitsHint,
Optional<RangeResult>* cache) const {
KeyRangeMap<Optional<RangeResult>>* cache) const {
return getRangeAsyncActor(this, ryw, kr, limitsHint, cache);
}
@ -135,17 +135,18 @@ public:
ReadYourWritesTransaction* ryw,
KeyRangeRef kr,
GetRangeLimits limits,
Optional<RangeResult>* cache) {
KeyRangeMap<Optional<RangeResult>>* cache) {
ASSERT(skrAyncImpl->getKeyRange().contains(kr));
ASSERT(cache != nullptr);
if (!cache->present()) {
ASSERT(cache->rangeContaining(kr.begin) == cache->rangeContainingKeyBefore(kr.end));
if (!(*cache)[kr.begin].present()) {
// For simplicity, every time we need to cache, we read the whole range
// Although sometimes the range can be narrowed,
// there is not a general way to do it in complicated scenarios
RangeResult result_ = wait(skrAyncImpl->getRange(ryw, skrAyncImpl->getKeyRange(), limits));
*cache = result_;
cache->insert(skrAyncImpl->getKeyRange(), result_);
}
const auto& allResults = cache->get();
const auto& allResults = (*cache)[kr.begin].get();
int start = 0, end = allResults.size();
while (start < allResults.size() && allResults[start].key < kr.begin)
++start;
@ -271,15 +272,23 @@ private:
};
// Used for SpecialKeySpaceCorrectnessWorkload
class SKSCTestImpl : public SpecialKeyRangeRWImpl {
class SKSCTestRWImpl : public SpecialKeyRangeRWImpl {
public:
explicit SKSCTestImpl(KeyRangeRef kr);
explicit SKSCTestRWImpl(KeyRangeRef kr);
Future<RangeResult> getRange(ReadYourWritesTransaction* ryw,
KeyRangeRef kr,
GetRangeLimits limitsHint) const override;
Future<Optional<std::string>> commit(ReadYourWritesTransaction* ryw) override;
};
class SKSCTestAsyncReadImpl : public SpecialKeyRangeAsyncImpl {
public:
explicit SKSCTestAsyncReadImpl(KeyRangeRef kr);
Future<RangeResult> getRange(ReadYourWritesTransaction* ryw,
KeyRangeRef kr,
GetRangeLimits limitsHint) const override;
};
// Use special key prefix "\xff\xff/transaction/conflicting_keys/<some_key>",
// to retrieve keys which caused latest not_committed(conflicting with another transaction) error.
// The returned key value pairs are interpreted as :

30
fdbserver/workloads/SpecialKeySpaceCorrectness.actor.cpp
View File

@ -39,8 +39,10 @@ struct SpecialKeySpaceCorrectnessWorkload : TestWorkload {
double testDuration, absoluteRandomProb, transactionsPerSecond;
PerfIntCounter wrongResults, keysCount;
Reference<ReadYourWritesTransaction> ryw; // used to store all populated data
std::vector<std::shared_ptr<SKSCTestImpl>> impls;
std::vector<std::shared_ptr<SKSCTestRWImpl>> rwImpls;
std::vector<std::shared_ptr<SKSCTestAsyncReadImpl>> asyncReadImpls;
Standalone<VectorRef<KeyRangeRef>> keys;
Standalone<VectorRef<KeyRangeRef>> rwKeys;
SpecialKeySpaceCorrectnessWorkload(WorkloadContext const& wcx)
: TestWorkload(wcx), wrongResults("Wrong Results"), keysCount("Number of generated keys") {
@ -81,12 +83,20 @@ struct SpecialKeySpaceCorrectnessWorkload : TestWorkload {
Key startKey(baseKey + "/");
Key endKey(baseKey + "/\xff");
self->keys.push_back_deep(self->keys.arena(), KeyRangeRef(startKey, endKey));
self->impls.push_back(std::make_shared<SKSCTestImpl>(KeyRangeRef(startKey, endKey)));
// Although there are already ranges registered, the testing range will replace them
cx->specialKeySpace->registerKeyRange(SpecialKeySpace::MODULE::TESTONLY,
SpecialKeySpace::IMPLTYPE::READWRITE,
self->keys.back(),
self->impls.back().get());
if (deterministicRandom()->random01() < 0.2) {
self->asyncReadImpls.push_back(std::make_shared<SKSCTestAsyncReadImpl>(KeyRangeRef(startKey, endKey)));
cx->specialKeySpace->registerKeyRange(SpecialKeySpace::MODULE::TESTONLY,
SpecialKeySpace::IMPLTYPE::READONLY,
self->keys.back(),
self->asyncReadImpls.back().get());
} else {
self->rwImpls.push_back(std::make_shared<SKSCTestRWImpl>(KeyRangeRef(startKey, endKey)));
// Although there are already ranges registered, the testing range will replace them
cx->specialKeySpace->registerKeyRange(SpecialKeySpace::MODULE::TESTONLY,
SpecialKeySpace::IMPLTYPE::READWRITE,
self->keys.back(),
self->rwImpls.back().get());
}
// generate keys in each key range
int keysInRange = deterministicRandom()->randomInt(self->minKeysPerRange, self->maxKeysPerRange + 1);
self->keysCount += keysInRange;
@ -154,7 +164,7 @@ struct SpecialKeySpaceCorrectnessWorkload : TestWorkload {
}
// check ryw result consistency
KeyRange rkr = self->randomKeyRange();
KeyRange rkr = self->randomRWKeyRange();
KeyRef rkey1 = rkr.begin;
KeyRef rkey2 = rkr.end;
// randomly set/clear two keys or clear a key range
@ -238,8 +248,8 @@ struct SpecialKeySpaceCorrectnessWorkload : TestWorkload {
return true;
}
KeyRange randomKeyRange() {
Key prefix = keys[deterministicRandom()->randomInt(0, rangeCount)].begin;
KeyRange randomRWKeyRange() {
Key prefix = rwImpls[deterministicRandom()->randomInt(0, rwImpls.size())]->getKeyRange().begin;
Key rkey1 = Key(deterministicRandom()->randomAlphaNumeric(deterministicRandom()->randomInt(0, keyBytes)))
.withPrefix(prefix);
Key rkey2 = Key(deterministicRandom()->randomAlphaNumeric(deterministicRandom()->randomInt(0, keyBytes)))