diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp index 9109a6e739..edb3317a05 100644 --- a/fdbclient/NativeAPI.actor.cpp +++ b/fdbclient/NativeAPI.actor.cpp @@ -529,6 +529,7 @@ struct WorkerInterfacesSpecialKeyImpl : SpecialKeyRangeBaseImpl { struct SingleSpecialKeyImpl : SpecialKeyRangeBaseImpl { Future> getRange(Reference ryw, KeyRangeRef kr) const override { + if (!kr.contains(k)) return Standalone(); return map(f(ryw), [k = k](Optional v) { Standalone result; if (v.present()) { @@ -583,7 +584,7 @@ DatabaseContext::DatabaseContext(Reference(specialKeys.begin, specialKeys.end)) { + specialKeySpace(std::make_unique(specialKeys.begin, specialKeys.end, /* test */ false)) { dbId = deterministicRandom()->randomUniqueID(); connected = clientInfo->get().proxies.size() ? Void() : clientInfo->onChange(); diff --git a/fdbclient/SpecialKeySpace.actor.cpp b/fdbclient/SpecialKeySpace.actor.cpp index 97663c61d9..c33db10268 100644 --- a/fdbclient/SpecialKeySpace.actor.cpp +++ b/fdbclient/SpecialKeySpace.actor.cpp @@ -23,7 +23,6 @@ #include "flow/actorcompiler.h" // This must be the last #include. std::unordered_map SpecialKeySpace::moduleToBoundary = { - { SpecialKeySpace::MODULE::TESTONLY, normalKeys }, // all test keys are supposed to be in normalKeys { SpecialKeySpace::MODULE::TRANSACTION, KeyRangeRef(LiteralStringRef("\xff\xff/transaction/"), LiteralStringRef("\xff\xff/transaction0")) }, { SpecialKeySpace::MODULE::WORKERINTERFACE, @@ -46,7 +45,7 @@ ACTOR Future moveKeySelectorOverRangeActor(const SpecialKeyRangeBaseImpl* if (ks->offset < 1) { // less than the given key - if (skrImpl->getKeyRange().contains(ks->getKey())) endKey = keyAfter(ks->getKey()); + if (skrImpl->getKeyRange().contains(ks->getKey())) endKey = ks->getKey(); // keyAfter(ks->getKey()); } else { // greater than the given key if (skrImpl->getKeyRange().contains(ks->getKey())) startKey = ks->getKey(); @@ -60,7 +59,7 @@ ACTOR Future moveKeySelectorOverRangeActor(const SpecialKeyRangeBaseImpl* Standalone result = wait(skrImpl->getRange(ryw, KeyRangeRef(startKey, endKey))); if (result.size() == 0) { - TraceEvent("ZeroElementsIntheRange").detail("Start", startKey).detail("End", endKey); + TraceEvent(SevDebug, "ZeroElementsIntheRange").detail("Start", startKey).detail("End", endKey); return Void(); } // Note : KeySelector::setKey has byte limit according to the knobs, customize it if needed @@ -110,9 +109,9 @@ ACTOR Future normalizeKeySelectorActor(SpecialKeySpace* sks, ReferencegetImpls().rangeContaining(ks->getKey()); while ((ks->offset < 1 && iter != sks->getImpls().ranges().begin()) || (ks->offset > 1 && iter != sks->getImpls().ranges().end())) { + onModuleRead(ryw, sks->getModules().rangeContaining(iter->begin())->value(), *lastModuleRead); if (iter->value() != nullptr) { wait(moveKeySelectorOverRangeActor(iter->value(), ryw, ks)); - onModuleRead(ryw, sks->getImplToModuleMap().at(iter->value()), *lastModuleRead); } ks->offset < 1 ? --iter : ++iter; } @@ -142,15 +141,16 @@ ACTOR Future> SpecialKeySpace::checkModuleFound(Speci GetRangeLimits limits, bool reverse) { std::pair, Optional> result = wait(SpecialKeySpace::getRangeAggregationActor(sks, ryw, begin, end, limits, reverse)); + // TODO : all cross_module_read or no_module_found error is better to check at the beginning, improve this later if (ryw && !ryw->specialKeySpaceRelaxed()) { auto module = result.second; if (!module.present()) { throw special_keys_no_module_found(); - } else { - auto boundary = sks->moduleToBoundary.at(module.get()); - if (!boundary.contains(begin.getKey()) || end.getKey() > boundary.end) - throw special_keys_cross_module_read(); - } + } /*else { + auto boundary = sks->moduleToBoundary.at(module.get()); + if (!boundary.contains(begin.getKey()) || end.getKey() < boundary.begin || end.getKey() > boundary.end) + throw special_keys_cross_module_read(); + }*/ } return result.first; } @@ -167,6 +167,7 @@ SpecialKeySpace::getRangeAggregationActor(SpecialKeySpace* sks, Reference lastModuleRead; wait(normalizeKeySelectorActor(sks, ryw, &begin, &lastModuleRead, &actualBeginOffset, &result)); + // TODO : check if end the boundary of a module wait(normalizeKeySelectorActor(sks, ryw, &end, &lastModuleRead, &actualEndOffset, &result)); // Handle all corner cases like what RYW does // return if range inverted @@ -187,8 +188,8 @@ SpecialKeySpace::getRangeAggregationActor(SpecialKeySpace* sks, ReferencegetModules().rangeContaining(iter->begin())->value(), lastModuleRead); if (iter->value() == nullptr) continue; - onModuleRead(ryw, sks->getImplToModuleMap().at(iter->value()), lastModuleRead); KeyRangeRef kr = iter->range(); KeyRef keyStart = kr.contains(begin.getKey()) ? begin.getKey() : kr.begin; KeyRef keyEnd = kr.contains(end.getKey()) ? end.getKey() : kr.end; @@ -210,8 +211,8 @@ SpecialKeySpace::getRangeAggregationActor(SpecialKeySpace* sks, ReferencegetModules().rangeContaining(iter->begin())->value(), lastModuleRead); if (iter->value() == nullptr) continue; - onModuleRead(ryw, sks->getImplToModuleMap().at(iter->value()), lastModuleRead); KeyRangeRef kr = iter->range(); KeyRef keyStart = kr.contains(begin.getKey()) ? begin.getKey() : kr.begin; KeyRef keyEnd = kr.contains(end.getKey()) ? end.getKey() : kr.end; @@ -248,6 +249,11 @@ Future> SpecialKeySpace::getRange(Reference= end.offset && begin.getKey() >= end.getKey() ) { + // TEST(true); // RYW range inverted + // return Standalone(); + // } + return checkModuleFound(this, ryw, begin, end, limits, reverse); } diff --git a/fdbclient/SpecialKeySpace.actor.h b/fdbclient/SpecialKeySpace.actor.h index ad2f15bb14..f4836c7cc3 100644 --- a/fdbclient/SpecialKeySpace.actor.h +++ b/fdbclient/SpecialKeySpace.actor.h @@ -51,6 +51,8 @@ protected: class SpecialKeySpace { public: enum class MODULE { + UNKNOWN, // used for all unregistered range, a cross_module_read will happen if your range read contains any + // UNKKNOWN module range TESTONLY, // only used by tests TRANSACTION, WORKERINTERFACE, @@ -64,23 +66,42 @@ public: Future> getRange(Reference ryw, KeySelector begin, KeySelector end, GetRangeLimits limits, bool reverse = false); - SpecialKeySpace(KeyRef spaceStartKey = Key(), KeyRef spaceEndKey = normalKeys.end) { + SpecialKeySpace(KeyRef spaceStartKey = Key(), KeyRef spaceEndKey = normalKeys.end, bool test = true) { // Default value is nullptr, begin of KeyRangeMap is Key() impls = KeyRangeMap(nullptr, spaceEndKey); range = KeyRangeRef(spaceStartKey, spaceEndKey); + modules = KeyRangeMap(SpecialKeySpace::MODULE::UNKNOWN, spaceEndKey); + if (!test) modulesBoundaryInit(); + // TODO : Handle testonly here + } + void modulesBoundaryInit() { + for (const auto& pair : moduleToBoundary) { + ASSERT(range.contains(pair.second)); + // Make sure the module is not overlapping with any registered modules + ASSERT(modules.rangeContaining(pair.second.begin) == modules.rangeContaining(pair.second.end) && + modules[pair.second.begin] == SpecialKeySpace::MODULE::UNKNOWN); + modules.insert(pair.second, pair.first); + impls.insert(pair.second, nullptr); + } } void registerKeyRange(SpecialKeySpace::MODULE module, const KeyRangeRef& kr, SpecialKeyRangeBaseImpl* impl) { // range check // TODO: add range check not to be replaced by overlapped ones - ASSERT(kr.begin >= range.begin && kr.end <= range.end); + ASSERT(range.contains(kr)); // make sure the registered range is not overlapping with existing ones // Note: kr.end should not be the same as another range's begin, although it should work even they are the same - ASSERT(impls.rangeContaining(kr.begin) == impls.rangeContaining(kr.end) && impls[kr.begin] == nullptr); + // ASSERT(impls.rangeContaining(kr.begin) == impls.rangeContaining(kr.end) && impls[kr.begin] == nullptr); + for (auto iter = impls.rangeContaining(kr.begin); true; ++iter) { + ASSERT(iter->value() == nullptr); + if (iter == impls.rangeContaining(kr.end)) break; // relax the end to be another one's start if needed + } impls.insert(kr, impl); // Set module for the range - implToModule[impl] = module; + implToModule[impl] = module; // TODO : check do we really need this map } + KeyRangeMap& getImpls() { return impls; } + KeyRangeMap& getModules() { return modules; } KeyRangeRef getKeyRange() const { return range; } const std::unordered_map& getImplToModuleMap() const { return implToModule; @@ -99,6 +120,7 @@ private: KeySelector end, GetRangeLimits limits, bool reverse); KeyRangeMap impls; + KeyRangeMap modules; KeyRange range; std::unordered_map implToModule; diff --git a/fdbserver/workloads/SpecialKeySpaceCorrectness.actor.cpp b/fdbserver/workloads/SpecialKeySpaceCorrectness.actor.cpp index 4ff0d020a2..9ef38f6c66 100644 --- a/fdbserver/workloads/SpecialKeySpaceCorrectness.actor.cpp +++ b/fdbserver/workloads/SpecialKeySpaceCorrectness.actor.cpp @@ -108,8 +108,8 @@ struct SpecialKeySpaceCorrectnessWorkload : TestWorkload { return Void(); } ACTOR Future _start(Database cx, SpecialKeySpaceCorrectnessWorkload* self) { - wait(timeout(self->getRangeCallActor(cx, self) && testConflictRanges(cx, /*read*/ true, self) && - testConflictRanges(cx, /*read*/ false, self), + wait(timeout(self->testCrossModuleRead(cx, self) && self->getRangeCallActor(cx, self) && + testConflictRanges(cx, /*read*/ true, self) && testConflictRanges(cx, /*read*/ false, self), self->testDuration, Void())); return Void(); } @@ -224,6 +224,58 @@ struct SpecialKeySpaceCorrectnessWorkload : TestWorkload { return GetRangeLimits(rowLimits, byteLimits); } + ACTOR Future testCrossModuleRead(Database cx_, SpecialKeySpaceCorrectnessWorkload* self) { + Database cx = cx_->clone(); + state Reference tx = Reference(new ReadYourWritesTransaction(cx)); + try { + wait(success(tx->getRange( + KeyRangeRef(LiteralStringRef("\xff\xff/transactio"), LiteralStringRef("\xff\xff/transaction0")), + CLIENT_KNOBS->TOO_MANY))); + ASSERT(false); + } catch (Error& e) { + if (e.code() == error_code_actor_cancelled) throw; + ASSERT(e.code() == error_code_special_keys_cross_module_read); + } + try { + wait(success(tx->getRange( + KeyRangeRef(LiteralStringRef("\xff\xff/transaction/"), LiteralStringRef("\xff\xff/transaction1")), + CLIENT_KNOBS->TOO_MANY))); + ASSERT(false); + } catch (Error& e) { + if (e.code() == error_code_actor_cancelled) throw; + ASSERT(e.code() == error_code_special_keys_cross_module_read); + } + try { + wait(success(tx->getRange( + KeyRangeRef(LiteralStringRef("\xff\xff/transaction"), LiteralStringRef("\xff\xff/transaction1")), + CLIENT_KNOBS->TOO_MANY))); + ASSERT(false); + } catch (Error& e) { + if (e.code() == error_code_actor_cancelled) throw; + ASSERT(e.code() == error_code_special_keys_cross_module_read); + } + try { + wait(success(tx->getRange( + KeyRangeRef(LiteralStringRef("\xff\xff/transaction/"), LiteralStringRef("\xff\xff/transaction0")), + CLIENT_KNOBS->TOO_MANY))); + ASSERT(true); + } catch (Error& e) { + throw; + } + + try { + const KeyRef key = LiteralStringRef("\xff\xff/cluster_file_path"); + KeySelector begin = KeySelectorRef(key, false, 0); + KeySelector end = KeySelectorRef(keyAfter(key), false, 1); + wait(success(tx->getRange(begin, end, GetRangeLimits(CLIENT_KNOBS->TOO_MANY)))); + ASSERT(false); + } catch (Error& e) { + if (e.code() == error_code_actor_cancelled) throw; + ASSERT(e.code() == error_code_special_keys_cross_module_read); + } + return Void(); + } + ACTOR static Future testConflictRanges(Database cx_, bool read, SpecialKeySpaceCorrectnessWorkload* self) { state StringRef prefix = read ? readConflictRangeKeysRange.begin : writeConflictRangeKeysRange.begin; TEST(read); // test read conflict range special key implementation