From 94e0c832142e671f7510b36bae9231b7a5015e0f Mon Sep 17 00:00:00 2001 From: Josh Slocum Date: Fri, 2 Sep 2022 11:03:21 -0500 Subject: [PATCH] Blob range idempotency test and fixes --- fdbcli/BlobRangeCommand.actor.cpp | 7 +++- fdbclient/KeyRangeMap.actor.cpp | 34 ++++++++++++++++--- fdbclient/NativeAPI.actor.cpp | 27 +++++++-------- .../BlobGranuleRangesWorkload.actor.cpp | 18 ++++++++-- 4 files changed, 63 insertions(+), 23 deletions(-) diff --git a/fdbcli/BlobRangeCommand.actor.cpp b/fdbcli/BlobRangeCommand.actor.cpp index 4c6bdf9614..b9b7217cf9 100644 --- a/fdbcli/BlobRangeCommand.actor.cpp +++ b/fdbcli/BlobRangeCommand.actor.cpp @@ -135,7 +135,12 @@ ACTOR Future blobRangeCommandActor(Database localDb, } else { wait(store(success, localDb->unblobbifyRange(KeyRangeRef(begin, end)))); } - if (!success) { + if (success) { + fmt::print("Successfully updated blob range [{0} - {1}) to {2}\n", + range.begin.printable(), + range.end.printable(), + value.printable()); + } else { fmt::print("{0} blobbify range for [{1} - {2}) failed\n", starting ? "Starting" : "Stopping", tokens[2].printable().c_str(), diff --git a/fdbclient/KeyRangeMap.actor.cpp b/fdbclient/KeyRangeMap.actor.cpp index cb1f0558c1..5ec7ffd713 100644 --- a/fdbclient/KeyRangeMap.actor.cpp +++ b/fdbclient/KeyRangeMap.actor.cpp @@ -127,7 +127,10 @@ ACTOR Future krmGetRangesUnaligned(Transaction* tr, state GetRangeLimits limits(limit, limitBytes); limits.minRows = 2; - RangeResult kv = wait(tr->getRange(lastLessOrEqual(withPrefix.begin), firstGreaterThan(withPrefix.end), limits)); + // wait to include the next highest row >= keys.end in the result, so since end is exclusive, we need +2 and + // !orEqual + RangeResult kv = + wait(tr->getRange(lastLessOrEqual(withPrefix.begin), KeySelectorRef(withPrefix.end, false, +2), limits)); return krmDecodeRanges(mapPrefix, keys, kv, false); } @@ -142,7 +145,10 @@ ACTOR Future krmGetRangesUnaligned(ReferencegetRange(lastLessOrEqual(withPrefix.begin), firstGreaterThan(withPrefix.end), limits)); + // wait to include the next highest row >= keys.end in the result, so since end is exclusive, we need +2 and + // !orEqual + RangeResult kv = + wait(tr->getRange(lastLessOrEqual(withPrefix.begin), KeySelectorRef(withPrefix.end, false, +2), limits)); return krmDecodeRanges(mapPrefix, keys, kv, false); } @@ -323,17 +329,27 @@ TEST_CASE("/keyrangemap/decoderange/aligned") { StringRef keyD = StringRef(arena, LiteralStringRef("d")); StringRef keyE = StringRef(arena, LiteralStringRef("e")); StringRef keyAB = StringRef(arena, LiteralStringRef("ab")); + StringRef keyAC = StringRef(arena, LiteralStringRef("ac")); StringRef keyCD = StringRef(arena, LiteralStringRef("cd")); // Fake getRange() call. RangeResult kv; kv.push_back(arena, KeyValueRef(fullKeyA, keyA)); kv.push_back(arena, KeyValueRef(fullKeyB, keyB)); + + // [A, AB(start), AC(start), B] + RangeResult decodedRanges = krmDecodeRanges(prefix, KeyRangeRef(keyAB, keyAC), kv); + ASSERT(decodedRanges.size() == 2); + ASSERT(decodedRanges.front().key == keyAB); + ASSERT(decodedRanges.front().value == keyA); + ASSERT(decodedRanges.back().key == keyAC); + ASSERT(decodedRanges.back().value == keyA); + kv.push_back(arena, KeyValueRef(fullKeyC, keyC)); kv.push_back(arena, KeyValueRef(fullKeyD, keyD)); // [A, AB(start), B, C, CD(end), D] - RangeResult decodedRanges = krmDecodeRanges(prefix, KeyRangeRef(keyAB, keyCD), kv); + decodedRanges = krmDecodeRanges(prefix, KeyRangeRef(keyAB, keyCD), kv); ASSERT(decodedRanges.size() == 4); ASSERT(decodedRanges.front().key == keyAB); ASSERT(decodedRanges.front().value == keyA); @@ -365,17 +381,27 @@ TEST_CASE("/keyrangemap/decoderange/unaligned") { StringRef keyD = StringRef(arena, LiteralStringRef("d")); StringRef keyE = StringRef(arena, LiteralStringRef("e")); StringRef keyAB = StringRef(arena, LiteralStringRef("ab")); + StringRef keyAC = StringRef(arena, LiteralStringRef("ac")); StringRef keyCD = StringRef(arena, LiteralStringRef("cd")); // Fake getRange() call. RangeResult kv; kv.push_back(arena, KeyValueRef(fullKeyA, keyA)); kv.push_back(arena, KeyValueRef(fullKeyB, keyB)); + + // [A, AB(start), AC(start), B] + RangeResult decodedRanges = krmDecodeRanges(prefix, KeyRangeRef(keyAB, keyAC), kv, false); + ASSERT(decodedRanges.size() == 2); + ASSERT(decodedRanges.front().key == keyA); + ASSERT(decodedRanges.front().value == keyA); + ASSERT(decodedRanges.back().key == keyB); + ASSERT(decodedRanges.back().value == keyB); + kv.push_back(arena, KeyValueRef(fullKeyC, keyC)); kv.push_back(arena, KeyValueRef(fullKeyD, keyD)); // [A, AB(start), B, C, CD(end), D] - RangeResult decodedRanges = krmDecodeRanges(prefix, KeyRangeRef(keyAB, keyCD), kv, false); + decodedRanges = krmDecodeRanges(prefix, KeyRangeRef(keyAB, keyCD), kv, false); ASSERT(decodedRanges.size() == 4); ASSERT(decodedRanges.front().key == keyA); ASSERT(decodedRanges.front().value == keyA); diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp index 1eaf7d7874..935e0aa13b 100644 --- a/fdbclient/NativeAPI.actor.cpp +++ b/fdbclient/NativeAPI.actor.cpp @@ -9987,31 +9987,32 @@ ACTOR Future setBlobRangeActor(Reference cx, KeyRange ran state Reference tr = makeReference(db); state Value value = active ? blobRangeActive : blobRangeInactive; - loop { try { tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); tr->setOption(FDBTransactionOptions::PRIORITY_SYSTEM_IMMEDIATE); - state Standalone> startBlobRanges = wait(getBlobRanges(tr, range, 10)); - state Standalone> endBlobRanges = - wait(getBlobRanges(tr, KeyRangeRef(range.end, keyAfter(range.end)), 10)); + Standalone> startBlobRanges = wait(getBlobRanges(tr, range, 1)); if (active) { // Idempotent request. - if (!startBlobRanges.empty() && !endBlobRanges.empty()) { - return startBlobRanges.front().begin == range.begin && endBlobRanges.front().end == range.end; + if (!startBlobRanges.empty()) { + return startBlobRanges.front().begin == range.begin && startBlobRanges.front().end == range.end; } } else { // An unblobbify request must be aligned to boundaries. // It is okay to unblobbify multiple regions all at once. - if (startBlobRanges.empty() && endBlobRanges.empty()) { + if (startBlobRanges.empty()) { + // already unblobbified return true; + } else if (startBlobRanges.front().begin != range.begin) { + // If there is a blob at the beginning of the range and it isn't aligned + return false; } - // If there is a blob at the beginning of the range and it isn't aligned, - // or there is a blob range that begins before the end of the range, then fail. - if ((!startBlobRanges.empty() && startBlobRanges.front().begin != range.begin) || - (!endBlobRanges.empty() && endBlobRanges.front().begin < range.end)) { + // if blob range does start at the specified, key, we need to make sure the end of also a boundary of a + // blob range + Optional endPresent = wait(tr->get(range.end.withPrefix(blobRangeKeys.begin))); + if (!endPresent.present()) { return false; } } @@ -10020,10 +10021,6 @@ ACTOR Future setBlobRangeActor(Reference cx, KeyRange ran // This is not coalescing because we want to keep each range logically separate. wait(krmSetRange(tr, blobRangeKeys.begin, range, value)); wait(tr->commit()); - printf("Successfully updated blob range [%s - %s) to %s\n", - range.begin.printable().c_str(), - range.end.printable().c_str(), - value.printable().c_str()); return true; } catch (Error& e) { wait(tr->onError(e)); diff --git a/fdbserver/workloads/BlobGranuleRangesWorkload.actor.cpp b/fdbserver/workloads/BlobGranuleRangesWorkload.actor.cpp index 050dfe7fe5..b72b884656 100644 --- a/fdbserver/workloads/BlobGranuleRangesWorkload.actor.cpp +++ b/fdbserver/workloads/BlobGranuleRangesWorkload.actor.cpp @@ -466,6 +466,16 @@ struct BlobGranuleRangesWorkload : TestWorkload { state Key middleKey = range.begin.withSuffix("AF"_sr); state Key middleKey2 = range.begin.withSuffix("AG"_sr); + if (BGRW_DEBUG) { + fmt::print("IdempotentUnit: [{0} - {1})\n", range.begin.printable(), range.end.printable()); + } + + // unblobbifying range that already doesn't exist should be no-op + if (deterministicRandom()->coinflip()) { + bool unblobbifyStartSuccess = wait(self->setRange(cx, activeRange, false)); + ASSERT(unblobbifyStartSuccess); + } + bool success = wait(self->setRange(cx, activeRange, true)); ASSERT(success); wait(self->checkRange(cx, self, activeRange, true)); @@ -544,8 +554,11 @@ struct BlobGranuleRangesWorkload : TestWorkload { bool unblobbifyFail8 = wait(self->setRange(cx, KeyRangeRef(middleKey, middleKey2), false)); ASSERT(!unblobbifyFail8); - bool unblobbifySuccess = wait(self->setRange(cx, activeRange, false)); - ASSERT(!unblobbifySuccess); + bool unblobbifySuccess = wait(self->setRange(cx, activeRange, true)); + ASSERT(unblobbifySuccess); + + bool unblobbifySuccessAgain = wait(self->setRange(cx, activeRange, true)); + ASSERT(unblobbifySuccessAgain); return Void(); } @@ -592,7 +605,6 @@ struct BlobGranuleRangesWorkload : TestWorkload { // FIXME: fix bugs and enable these tests! excludedTypes.insert(RANGES_MISALIGNED); // TODO - fix in blob manager - excludedTypes.insert(BLOBBIFY_IDEMPOTENT); // fix already in progress in a separate PR excludedTypes.insert(RE_BLOBBIFY); // TODO - fix is non-trivial, is desired behavior eventually std::string nextRangeKey = "U_" + self->newKey();