Merge pull request #7321 from sfc-gh-ajbeamon/multiple-tenant-creation

Support creating multiple tenants in the same transaction

fdbclient/GenericManagementAPI.actor.h

@@ -671,7 +671,6 @@ Future<Optional<TenantMapEntry>> tryGetTenantTransaction(Transaction tr, TenantN
 	state Key tenantMapKey = name.withPrefix(tenantMapPrefix);
 
 	tr->setOption(FDBTransactionOptions::RAW_ACCESS);
-	tr->setOption(FDBTransactionOptions::READ_LOCK_AWARE);
 
 	state typename transaction_future_type<Transaction, Optional<Value>>::type tenantFuture = tr->get(tenantMapKey);
 	Optional<Value> val = wait(safeThreadFutureToFuture(tenantFuture));
@@ -685,6 +684,7 @@ Future<Optional<TenantMapEntry>> tryGetTenant(Reference<DB> db, TenantName name)
 	loop {
 		try {
 			tr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS);
+			tr->setOption(FDBTransactionOptions::READ_LOCK_AWARE);
 			Optional<TenantMapEntry> entry = wait(tryGetTenantTransaction(tr, name));
 			return entry;
 		} catch (Error& e) {
@@ -714,8 +714,10 @@ Future<TenantMapEntry> getTenant(Reference<DB> db, TenantName name) {
 }
 
 // Creates a tenant with the given name. If the tenant already exists, an empty optional will be returned.
+// The caller must enforce that the tenant ID be unique from all current and past tenants, and it must also be unique
+// from all other tenants created in the same transaction.
 ACTOR template <class Transaction>
-Future<Optional<TenantMapEntry>> createTenantTransaction(Transaction tr, TenantNameRef name) {
+Future<std::pair<TenantMapEntry, bool>> createTenantTransaction(Transaction tr, TenantNameRef name, int64_t tenantId) {
 	state Key tenantMapKey = name.withPrefix(tenantMapPrefix);
 
 	if (name.startsWith("\xff"_sr)) {
@@ -723,12 +725,10 @@ Future<Optional<TenantMapEntry>> createTenantTransaction(Transaction tr, TenantN
 	}
 
 	tr->setOption(FDBTransactionOptions::RAW_ACCESS);
-	tr->setOption(FDBTransactionOptions::LOCK_AWARE);
 
 	state Future<Optional<TenantMapEntry>> tenantEntryFuture = tryGetTenantTransaction(tr, name);
 	state typename transaction_future_type<Transaction, Optional<Value>>::type tenantDataPrefixFuture =
 	    tr->get(tenantDataPrefixKey);
-	state typename transaction_future_type<Transaction, Optional<Value>>::type lastIdFuture = tr->get(tenantLastIdKey);
 	state typename transaction_future_type<Transaction, Optional<Value>>::type tenantModeFuture =
 	    tr->get(configKeysPrefix.withSuffix("tenant_mode"_sr));
 
@@ -740,12 +740,10 @@ Future<Optional<TenantMapEntry>> createTenantTransaction(Transaction tr, TenantN
 
 	Optional<TenantMapEntry> tenantEntry = wait(tenantEntryFuture);
 	if (tenantEntry.present()) {
-		return Optional<TenantMapEntry>();
+		return std::make_pair(tenantEntry.get(), false);
 	}
 
-	state Optional<Value> lastIdVal = wait(safeThreadFutureToFuture(lastIdFuture));
 	Optional<Value> tenantDataPrefix = wait(safeThreadFutureToFuture(tenantDataPrefixFuture));
-
 	if (tenantDataPrefix.present() &&
 	    tenantDataPrefix.get().size() + TenantMapEntry::ROOT_PREFIX_SIZE > CLIENT_KNOBS->TENANT_PREFIX_SIZE_LIMIT) {
 		TraceEvent(SevWarnAlways, "TenantPrefixTooLarge")
@@ -757,8 +755,7 @@ Future<Optional<TenantMapEntry>> createTenantTransaction(Transaction tr, TenantN
 		throw client_invalid_operation();
 	}
 
-	state TenantMapEntry newTenant(lastIdVal.present() ? TenantMapEntry::prefixToId(lastIdVal.get()) + 1 : 0,
+	state TenantMapEntry newTenant(tenantId, tenantDataPrefix.present() ? (KeyRef)tenantDataPrefix.get() : ""_sr);
-	                               tenantDataPrefix.present() ? (KeyRef)tenantDataPrefix.get() : ""_sr);
 
 	state typename transaction_future_type<Transaction, RangeResult>::type prefixRangeFuture =
 	    tr->getRange(prefixRange(newTenant.prefix), 1);
@@ -767,20 +764,21 @@ Future<Optional<TenantMapEntry>> createTenantTransaction(Transaction tr, TenantN
 		throw tenant_prefix_allocator_conflict();
 	}
 
-	tr->set(tenantLastIdKey, TenantMapEntry::idToPrefix(newTenant.id));
 	tr->set(tenantMapKey, encodeTenantEntry(newTenant));
 
-	return newTenant;
+	return std::make_pair(newTenant, true);
 }
 
 ACTOR template <class DB>
-Future<Void> createTenant(Reference<DB> db, TenantName name) {
+Future<TenantMapEntry> createTenant(Reference<DB> db, TenantName name) {
 	state Reference<typename DB::TransactionT> tr = db->createTransaction();
 
 	state bool firstTry = true;
 	loop {
 		try {
 			tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
+			tr->setOption(FDBTransactionOptions::LOCK_AWARE);
+			state typename DB::TransactionT::template FutureT<Optional<Value>> lastIdFuture = tr->get(tenantLastIdKey);
 
 			if (firstTry) {
 				Optional<TenantMapEntry> entry = wait(tryGetTenantTransaction(tr, name));
@@ -791,7 +789,10 @@ Future<Void> createTenant(Reference<DB> db, TenantName name) {
 				firstTry = false;
 			}
 
-			state Optional<TenantMapEntry> newTenant = wait(createTenantTransaction(tr, name));
+			Optional<Value> lastIdVal = wait(safeThreadFutureToFuture(lastIdFuture));
+			int64_t tenantId = lastIdVal.present() ? TenantMapEntry::prefixToId(lastIdVal.get()) + 1 : 0;
+			tr->set(tenantLastIdKey, TenantMapEntry::idToPrefix(tenantId));
+			state std::pair<TenantMapEntry, bool> newTenant = wait(createTenantTransaction(tr, name, tenantId));
 
 			if (BUGGIFY) {
 				throw commit_unknown_result();
@@ -805,11 +806,11 @@ Future<Void> createTenant(Reference<DB> db, TenantName name) {
 
 			TraceEvent("CreatedTenant")
 			    .detail("Tenant", name)
-			    .detail("TenantId", newTenant.present() ? newTenant.get().id : -1)
+			    .detail("TenantId", newTenant.first.id)
-			    .detail("Prefix", newTenant.present() ? (StringRef)newTenant.get().prefix : "Unknown"_sr)
+			    .detail("Prefix", newTenant.first.prefix)
 			    .detail("Version", tr->getCommittedVersion());
 
-			return Void();
+			return newTenant.first;
 		} catch (Error& e) {
 			wait(safeThreadFutureToFuture(tr->onError(e)));
 		}
@@ -821,7 +822,6 @@ Future<Void> deleteTenantTransaction(Transaction tr, TenantNameRef name) {
 	state Key tenantMapKey = name.withPrefix(tenantMapPrefix);
 
 	tr->setOption(FDBTransactionOptions::RAW_ACCESS);
-	tr->setOption(FDBTransactionOptions::LOCK_AWARE);
 
 	state Optional<TenantMapEntry> tenantEntry = wait(tryGetTenantTransaction(tr, name));
 	if (!tenantEntry.present()) {
@@ -848,6 +848,7 @@ Future<Void> deleteTenant(Reference<DB> db, TenantName name) {
 	loop {
 		try {
 			tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
+			tr->setOption(FDBTransactionOptions::LOCK_AWARE);
 
 			if (firstTry) {
 				Optional<TenantMapEntry> entry = wait(tryGetTenantTransaction(tr, name));
@@ -886,7 +887,6 @@ Future<std::map<TenantName, TenantMapEntry>> listTenantsTransaction(Transaction
 	state KeyRange range = KeyRangeRef(begin, end).withPrefix(tenantMapPrefix);
 
 	tr->setOption(FDBTransactionOptions::RAW_ACCESS);
-	tr->setOption(FDBTransactionOptions::READ_LOCK_AWARE);
 
 	state typename transaction_future_type<Transaction, RangeResult>::type listFuture =
 	    tr->getRange(firstGreaterOrEqual(range.begin), firstGreaterOrEqual(range.end), limit);
@@ -910,6 +910,7 @@ Future<std::map<TenantName, TenantMapEntry>> listTenants(Reference<DB> db,
 	loop {
 		try {
 			tr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS);
+			tr->setOption(FDBTransactionOptions::READ_LOCK_AWARE);
 			std::map<TenantName, TenantMapEntry> tenants = wait(listTenantsTransaction(tr, begin, end, limit));
 			return tenants;
 		} catch (Error& e) {

fdbclient/SpecialKeySpace.actor.cpp

@@ -2772,7 +2772,24 @@ Future<RangeResult> TenantMapRangeImpl::getRange(ReadYourWritesTransaction* ryw,
 	return getTenantList(ryw, kr, limitsHint);
 }
 
-ACTOR Future<Void> deleteTenantRange(ReadYourWritesTransaction* ryw, TenantName beginTenant, TenantName endTenant) {
+ACTOR Future<Void> createTenants(ReadYourWritesTransaction* ryw, std::vector<TenantNameRef> tenants) {
+	Optional<Value> lastIdVal = wait(ryw->getTransaction().get(tenantLastIdKey));
+	int64_t previousId = lastIdVal.present() ? TenantMapEntry::prefixToId(lastIdVal.get()) : -1;
+
+	std::vector<Future<Void>> createFutures;
+	for (auto tenant : tenants) {
+		createFutures.push_back(
+		    success(ManagementAPI::createTenantTransaction(&ryw->getTransaction(), tenant, ++previousId)));
+	}
+
+	ryw->getTransaction().set(tenantLastIdKey, TenantMapEntry::idToPrefix(previousId));
+	wait(waitForAll(createFutures));
+	return Void();
+}
+
+ACTOR Future<Void> deleteTenantRange(ReadYourWritesTransaction* ryw,
+                                     TenantNameRef beginTenant,
+                                     TenantNameRef endTenant) {
 	std::map<TenantName, TenantMapEntry> tenants = wait(
 	    ManagementAPI::listTenantsTransaction(&ryw->getTransaction(), beginTenant, endTenant, CLIENT_KNOBS->TOO_MANY));
 
@@ -2795,6 +2812,7 @@ ACTOR Future<Void> deleteTenantRange(ReadYourWritesTransaction* ryw, TenantName
 
 Future<Optional<std::string>> TenantMapRangeImpl::commit(ReadYourWritesTransaction* ryw) {
 	auto ranges = ryw->getSpecialKeySpaceWriteMap().containedRanges(range);
+	std::vector<TenantNameRef> tenantsToCreate;
 	std::vector<Future<Void>> tenantManagementFutures;
 	for (auto range : ranges) {
 		if (!range.value().first) {
@@ -2807,8 +2825,7 @@ Future<Optional<std::string>> TenantMapRangeImpl::commit(ReadYourWritesTransacti
 		        .removePrefix(TenantMapRangeImpl::submoduleRange.begin);
 
 		if (range.value().second.present()) {
-			tenantManagementFutures.push_back(
+			tenantsToCreate.push_back(tenantName);
-			    success(ManagementAPI::createTenantTransaction(&ryw->getTransaction(), tenantName)));
 		} else {
 			// For a single key clear, just issue the delete
 			if (KeyRangeRef(range.begin(), range.end()).singleKeyRange()) {
@@ -2827,5 +2844,9 @@ Future<Optional<std::string>> TenantMapRangeImpl::commit(ReadYourWritesTransacti
 		}
 	}
 
+	if (tenantsToCreate.size()) {
+		tenantManagementFutures.push_back(createTenants(ryw, tenantsToCreate));
+	}
+
 	return tag(waitForAll(tenantManagementFutures), Optional<std::string>());
 }

fdbserver/tester.actor.cpp

@@ -1629,7 +1629,7 @@ ACTOR Future<Void> runTests(Reference<AsyncVar<Optional<struct ClusterController
 		std::vector<Future<Void>> tenantFutures;
 		for (auto tenant : tenantsToCreate) {
 			TraceEvent("CreatingTenant").detail("Tenant", tenant);
-			tenantFutures.push_back(ManagementAPI::createTenant(cx.getReference(), tenant));
+			tenantFutures.push_back(success(ManagementAPI::createTenant(cx.getReference(), tenant)));
 		}
 
 		wait(waitForAll(tenantFutures));

fdbserver/workloads/BlobGranuleCorrectnessWorkload.actor.cpp

@@ -206,30 +206,14 @@ struct BlobGranuleCorrectnessWorkload : TestWorkload {
 		if (BGW_DEBUG) {
 			fmt::print("Setting up blob granule range for tenant {0}\n", name.printable());
 		}
-		state Reference<ReadYourWritesTransaction> tr = makeReference<ReadYourWritesTransaction>(cx);
-		loop {
-			try {
-				tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
-				tr->setOption(FDBTransactionOptions::PRIORITY_SYSTEM_IMMEDIATE);
 
-				state Optional<TenantMapEntry> entry = wait(ManagementAPI::createTenantTransaction(tr, name));
+		TenantMapEntry entry = wait(ManagementAPI::createTenant(cx.getReference(), name));
-				if (!entry.present()) {
-					// if tenant already exists because of retry, load it
-					wait(store(entry, ManagementAPI::tryGetTenantTransaction(tr, name)));
-					ASSERT(entry.present());
-				}
 
-				wait(tr->commit());
+		if (BGW_DEBUG) {
-				if (BGW_DEBUG) {
+			fmt::print("Set up blob granule range for tenant {0}: {1}\n", name.printable(), entry.prefix.printable());
-					fmt::print("Set up blob granule range for tenant {0}: {1}\n",
-					           name.printable(),
-					           entry.get().prefix.printable());
-				}
-				return entry.get();
-			} catch (Error& e) {
-				wait(tr->onError(e));
-			}
 		}
+
+		return entry;
 	}
 
 	std::string description() const override { return "BlobGranuleCorrectnessWorkload"; }

fdbserver/workloads/FuzzApiCorrectness.actor.cpp

@@ -225,7 +225,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload {
 
 			// The last tenant will not be created
 			if (i < self->numTenants) {
-				tenantFutures.push_back(ManagementAPI::createTenant(cx.getReference(), tenantName));
+				tenantFutures.push_back(::success(ManagementAPI::createTenant(cx.getReference(), tenantName)));
 				self->createdTenants.insert(tenantName);
 			}
 		}

fdbserver/workloads/TenantManagement.actor.cpp

@@ -131,88 +131,129 @@ struct TenantManagementWorkload : TestWorkload {
 	}
 
 	ACTOR Future<Void> createTenant(Database cx, TenantManagementWorkload* self) {
-		state TenantName tenant = self->chooseTenantName(true);
-		state bool alreadyExists = self->createdTenants.count(tenant);
 		state OperationType operationType = TenantManagementWorkload::randomOperationType();
+		int numTenants = 1;
+
+		// For transaction-based operations, test creating multiple tenants in the same transaction
+		if (operationType == OperationType::SPECIAL_KEYS || operationType == OperationType::MANAGEMENT_TRANSACTION) {
+			numTenants = deterministicRandom()->randomInt(1, 5);
+		}
+
+		state bool alreadyExists = false;
+		state bool hasSystemTenant = false;
+
+		state std::set<TenantName> tenantsToCreate;
+		for (int i = 0; i < numTenants; ++i) {
+			TenantName tenant = self->chooseTenantName(true);
+			tenantsToCreate.insert(tenant);
+
+			alreadyExists = alreadyExists || self->createdTenants.count(tenant);
+			hasSystemTenant = hasSystemTenant || tenant.startsWith("\xff"_sr);
+		}
+
 		state Reference<ReadYourWritesTransaction> tr = makeReference<ReadYourWritesTransaction>(cx);
 
 		loop {
 			try {
 				if (operationType == OperationType::SPECIAL_KEYS) {
 					tr->setOption(FDBTransactionOptions::SPECIAL_KEY_SPACE_ENABLE_WRITES);
-					Key key = self->specialKeysTenantMapPrefix.withSuffix(tenant);
+					for (auto tenant : tenantsToCreate) {
-					tr->set(key, ""_sr);
+						tr->set(self->specialKeysTenantMapPrefix.withSuffix(tenant), ""_sr);
+					}
 					wait(tr->commit());
 				} else if (operationType == OperationType::MANAGEMENT_DATABASE) {
-					wait(ManagementAPI::createTenant(cx.getReference(), tenant));
+					ASSERT(tenantsToCreate.size() == 1);
+					wait(success(ManagementAPI::createTenant(cx.getReference(), *tenantsToCreate.begin())));
 				} else {
 					tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
-					Optional<TenantMapEntry> _ = wait(ManagementAPI::createTenantTransaction(tr, tenant));
+
+					Optional<Value> lastIdVal = wait(tr->get(tenantLastIdKey));
+					int64_t previousId = lastIdVal.present() ? TenantMapEntry::prefixToId(lastIdVal.get()) : -1;
+
+					std::vector<Future<Void>> createFutures;
+					for (auto tenant : tenantsToCreate) {
+						createFutures.push_back(
+						    success(ManagementAPI::createTenantTransaction(tr, tenant, ++previousId)));
+					}
+					tr->set(tenantLastIdKey, TenantMapEntry::idToPrefix(previousId));
+					wait(waitForAll(createFutures));
 					wait(tr->commit());
 				}
 
-				if (operationType != OperationType::MANAGEMENT_DATABASE && alreadyExists) {
+				if (operationType == OperationType::MANAGEMENT_DATABASE) {
-					return Void();
+					ASSERT(!alreadyExists);
 				}
 
-				ASSERT(!alreadyExists);
+				ASSERT(!hasSystemTenant);
-				ASSERT(!tenant.startsWith("\xff"_sr));
 
-				state Optional<TenantMapEntry> entry = wait(ManagementAPI::tryGetTenant(cx.getReference(), tenant));
+				state std::set<TenantName>::iterator tenantItr;
-				ASSERT(entry.present());
+				for (tenantItr = tenantsToCreate.begin(); tenantItr != tenantsToCreate.end(); ++tenantItr) {
-				ASSERT(entry.get().id > self->maxId);
+					if (self->createdTenants.count(*tenantItr)) {
-				ASSERT(entry.get().prefix.startsWith(self->tenantSubspace));
+						continue;
+					}
 
-				self->maxId = entry.get().id;
+					state Optional<TenantMapEntry> entry =
-				self->createdTenants[tenant] = TenantState(entry.get().id, true);
+					    wait(ManagementAPI::tryGetTenant(cx.getReference(), *tenantItr));
+					ASSERT(entry.present());
+					ASSERT(entry.get().id > self->maxId);
+					ASSERT(entry.get().prefix.startsWith(self->tenantSubspace));
 
-				state bool insertData = deterministicRandom()->random01() < 0.5;
+					self->maxId = entry.get().id;
-				if (insertData) {
+					self->createdTenants[*tenantItr] = TenantState(entry.get().id, true);
-					state Transaction insertTr(cx, tenant);
+
-					loop {
+					state bool insertData = deterministicRandom()->random01() < 0.5;
-						try {
+					if (insertData) {
-							insertTr.set(self->keyName, tenant);
+						state Transaction insertTr(cx, *tenantItr);
-							wait(insertTr.commit());
+						loop {
-							break;
+							try {
-						} catch (Error& e) {
+								insertTr.set(self->keyName, *tenantItr);
-							wait(insertTr.onError(e));
+								wait(insertTr.commit());
+								break;
+							} catch (Error& e) {
+								wait(insertTr.onError(e));
+							}
+						}
+
+						self->createdTenants[*tenantItr].empty = false;
+
+						state Transaction checkTr(cx);
+						loop {
+							try {
+								checkTr.setOption(FDBTransactionOptions::RAW_ACCESS);
+								Optional<Value> val = wait(checkTr.get(self->keyName.withPrefix(entry.get().prefix)));
+								ASSERT(val.present());
+								ASSERT(val.get() == *tenantItr);
+								break;
+							} catch (Error& e) {
+								wait(checkTr.onError(e));
+							}
 						}
 					}
 
-					self->createdTenants[tenant].empty = false;
+					wait(self->checkTenant(cx, self, *tenantItr, self->createdTenants[*tenantItr]));
-
-					state Transaction checkTr(cx);
-					loop {
-						try {
-							checkTr.setOption(FDBTransactionOptions::RAW_ACCESS);
-							Optional<Value> val = wait(checkTr.get(self->keyName.withPrefix(entry.get().prefix)));
-							ASSERT(val.present());
-							ASSERT(val.get() == tenant);
-							break;
-						} catch (Error& e) {
-							wait(checkTr.onError(e));
-						}
-					}
 				}
-
-				wait(self->checkTenant(cx, self, tenant, self->createdTenants[tenant]));
 				return Void();
 			} catch (Error& e) {
 				if (e.code() == error_code_invalid_tenant_name) {
-					ASSERT(tenant.startsWith("\xff"_sr));
+					ASSERT(hasSystemTenant);
 					return Void();
 				} else if (operationType == OperationType::MANAGEMENT_DATABASE) {
 					if (e.code() == error_code_tenant_already_exists) {
 						ASSERT(alreadyExists && operationType == OperationType::MANAGEMENT_DATABASE);
 					} else {
-						TraceEvent(SevError, "CreateTenantFailure").error(e).detail("TenantName", tenant);
+						ASSERT(tenantsToCreate.size() == 1);
+						TraceEvent(SevError, "CreateTenantFailure")
+						    .error(e)
+						    .detail("TenantName", *tenantsToCreate.begin());
 					}
 					return Void();
 				} else {
 					try {
 						wait(tr->onError(e));
 					} catch (Error& e) {
-						TraceEvent(SevError, "CreateTenantFailure").error(e).detail("TenantName", tenant);
+						for (auto tenant : tenantsToCreate) {
+							TraceEvent(SevError, "CreateTenantFailure").error(e).detail("TenantName", tenant);
+						}
 						return Void();
 					}
 				}