Blob range idempotency test and fixes

fdbcli/BlobRangeCommand.actor.cpp

@@ -135,7 +135,12 @@ ACTOR Future<bool> 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(),

fdbclient/KeyRangeMap.actor.cpp

@@ -127,7 +127,10 @@ ACTOR Future<RangeResult> 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<RangeResult> krmGetRangesUnaligned(Reference<ReadYourWritesTransact
 
 	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);
 }
@@ -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);

fdbclient/NativeAPI.actor.cpp

@@ -9987,31 +9987,32 @@ ACTOR Future<bool> setBlobRangeActor(Reference<DatabaseContext> cx, KeyRange ran
 	state Reference<ReadYourWritesTransaction> tr = makeReference<ReadYourWritesTransaction>(db);
 
 	state Value value = active ? blobRangeActive : blobRangeInactive;
-
 	loop {
 		try {
 			tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
 			tr->setOption(FDBTransactionOptions::PRIORITY_SYSTEM_IMMEDIATE);
 
-			state Standalone<VectorRef<KeyRangeRef>> startBlobRanges = wait(getBlobRanges(tr, range, 10));
+			Standalone<VectorRef<KeyRangeRef>> startBlobRanges = wait(getBlobRanges(tr, range, 1));
-			state Standalone<VectorRef<KeyRangeRef>> endBlobRanges =
-			    wait(getBlobRanges(tr, KeyRangeRef(range.end, keyAfter(range.end)), 10));
 
 			if (active) {
 				// Idempotent request.
-				if (!startBlobRanges.empty() && !endBlobRanges.empty()) {
+				if (!startBlobRanges.empty()) {
-					return startBlobRanges.front().begin == range.begin && endBlobRanges.front().end == range.end;
+					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,
+				// if blob range does start at the specified, key, we need to make sure the end of also a boundary of a
-				// or there is a blob range that begins before the end of the range, then fail.
+				// blob range
-				if ((!startBlobRanges.empty() && startBlobRanges.front().begin != range.begin) ||
+				Optional<Value> endPresent = wait(tr->get(range.end.withPrefix(blobRangeKeys.begin)));
-				    (!endBlobRanges.empty() && endBlobRanges.front().begin < range.end)) {
+				if (!endPresent.present()) {
 					return false;
 				}
 			}
@@ -10020,10 +10021,6 @@ ACTOR Future<bool> setBlobRangeActor(Reference<DatabaseContext> 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));

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));
+		bool unblobbifySuccess = wait(self->setRange(cx, activeRange, true));
-		ASSERT(!unblobbifySuccess);
+		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();