Expand support for tenant groups. Track where each tenant group is assigned and used groups to help assign tenants. Fix tenant state tracking bugs. Add help hints for metacluster remove FORCE.

fdbcli/MetaclusterCommands.actor.cpp

@@ -270,9 +270,16 @@ std::vector<const char*> metaclusterHintGenerator(std::vector<StringRef> const&
 			"<NAME>", "<max_tenant_groups=<NUM_GROUPS>|connection_string=<CONNECTION_STRING>>"
 		};
 		return std::vector<const char*>(opts.begin() + std::min<int>(1, tokens.size() - 2), opts.end());
-	} else if (tokencmp(tokens[1], "remove") && tokens.size() < 3) {
-		static std::vector<const char*> opts = { "<NAME>" };
-		return std::vector<const char*>(opts.begin() + tokens.size() - 2, opts.end());
+	} else if (tokencmp(tokens[1], "remove") && tokens.size() < 4) {
+		static std::vector<const char*> opts = { "[FORCE]", "<NAME>" };
+		if (tokens.size() == 2) {
+			return opts;
+		} else if (tokens.size() == 3 && (inArgument || tokens[2].size() == "FORCE"_sr.size()) &&
+		           "FORCE"_sr.startsWith(tokens[2])) {
+			return std::vector<const char*>(opts.begin() + tokens.size() - 2, opts.end());
+		} else {
+			return std::vector<const char*>();
+		}
 	} else if (tokencmp(tokens[1], "list") && tokens.size() < 5) {
 		static std::vector<const char*> opts = { "[BEGIN]", "[END]", "[LIMIT]" };
 		return std::vector<const char*>(opts.begin() + tokens.size() - 2, opts.end());

fdbclient/Metacluster.h

@@ -121,4 +121,26 @@ struct DataClusterRegistrationEntry {
 	}
 };
 
+struct TenantGroupEntry {
+	constexpr static FileIdentifier file_identifier = 10764222;
+
+	ClusterName assignedCluster;
+
+	TenantGroupEntry() = default;
+	TenantGroupEntry(ClusterName assignedCluster) : assignedCluster(assignedCluster) {}
+
+	Value encode() { return ObjectWriter::toValue(*this, IncludeVersion(ProtocolVersion::withMetacluster())); }
+	static TenantGroupEntry decode(ValueRef const& value) {
+		TenantGroupEntry entry;
+		ObjectReader reader(value.begin(), IncludeVersion());
+		reader.deserialize(entry);
+		return entry;
+	}
+
+	template <class Ar>
+	void serialize(Ar& ar) {
+		serializer(ar, assignedCluster);
+	}
+};
+
 #endif

fdbclient/MetaclusterManagement.actor.h

@@ -610,15 +610,40 @@ Future<std::map<ClusterName, DataClusterMetadata>> listClusters(Reference<DB> db
 
 ACTOR template <class Transaction>
 Future<std::pair<ClusterName, DataClusterMetadata>> assignTenant(Transaction tr, TenantMapEntry tenantEntry) {
+	// TODO: check for invalid tenant group name
+	// TODO: check that the chosen cluster is available, otherwise we can try another
+
+	state typename transaction_future_type<Transaction, Optional<Value>>::type groupMetadataFuture;
+	state bool creatingTenantGroup = true;
+	if (tenantEntry.tenantGroup.present()) {
+		groupMetadataFuture = tr->get(tenantGroupMetadataKeys.begin.withSuffix(tenantEntry.tenantGroup.get()));
+		Optional<Value> groupMetadata = wait(safeThreadFutureToFuture(groupMetadataFuture));
+		if (groupMetadata.present()) {
+			creatingTenantGroup = false;
+			state TenantGroupEntry groupEntry = TenantGroupEntry::decode(groupMetadata.get());
+			Optional<DataClusterMetadata> clusterMetadata =
+			    wait(tryGetClusterTransaction(tr, groupEntry.assignedCluster));
+
+			// TODO: This is only true if we clean up tenant state after force removal.
+			ASSERT(clusterMetadata.present());
+			return std::make_pair(groupEntry.assignedCluster, clusterMetadata.get());
+		}
+	}
 
 	// TODO: more efficient
-	// TODO: account for tenant groups
 	std::map<ClusterName, DataClusterMetadata> clusters =
 	    wait(listClustersTransaction(tr, ""_sr, "\xff"_sr, CLIENT_KNOBS->TOO_MANY));
 
 	for (auto c : clusters) {
-		if (c.second.entry.hasCapacity()) {
-			// TODO: check that the chosen cluster is available, otherwise we can try another
+		if (!creatingTenantGroup || c.second.entry.hasCapacity()) {
+			if (creatingTenantGroup) {
+				++c.second.entry.allocated.numTenantGroups;
+				updateClusterMetadata(tr, c.first, Optional<ClusterConnectionString>(), c.second.entry);
+				if (tenantEntry.tenantGroup.present()) {
+					tr->set(tenantGroupMetadataKeys.begin.withSuffix(tenantEntry.tenantGroup.get()),
+					        TenantGroupEntry(c.first).encode());
+				}
+			}
 			return c;
 		}
 	}
@@ -662,15 +687,9 @@ Future<Void> createTenant(Reference<DB> db, TenantName name, TenantMapEntry tena
 					clusterMetadata = actualMetadata.get();
 				}
 			} else {
-				// TODO: this should actually track the groups, not the tenants
-				++clusterMetadata.entry.allocated.numTenantGroups;
-				updateClusterMetadata(assignTr,
-				                      createdTenant.assignedCluster.get(),
-				                      Optional<ClusterConnectionString>(),
-				                      clusterMetadata.entry);
+				wait(safeThreadFutureToFuture(assignTr->commit()));
 			}
 
-			wait(safeThreadFutureToFuture(assignTr->commit()));
 			break;
 		} catch (Error& e) {
 			wait(safeThreadFutureToFuture(assignTr->onError(e)));
@@ -681,7 +700,6 @@ Future<Void> createTenant(Reference<DB> db, TenantName name, TenantMapEntry tena
 	state Reference<IDatabase> dataClusterDb = MultiVersionApi::api->createDatabase(
 	    makeReference<ClusterConnectionMemoryRecord>(clusterMetadata.connectionString));
 
-	createdTenant.tenantState = TenantState::READY;
 	TenantMapEntry _ = wait(ManagementAPI::createTenant(
 	    dataClusterDb, name, createdTenant, ManagementAPI::TenantOperationType::DATA_CLUSTER));
 
@@ -763,13 +781,33 @@ Future<Void> deleteTenant(Reference<DB> db, TenantName name) {
 			removeTr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
 			state Optional<TenantMapEntry> finalEntry = wait(ManagementAPI::tryGetTenantTransaction(removeTr, name));
 
+			wait(ManagementAPI::deleteTenantTransaction(
+			    removeTr, name, ManagementAPI::TenantOperationType::MANAGEMENT_CLUSTER));
+
 			if (finalEntry.present() && finalEntry.get().assignedCluster.present()) {
+				state typename DB::TransactionT::template FutureT<RangeResult> tenantGroupIndexFuture;
+				if (finalEntry.get().tenantGroup.present()) {
+					tenantGroupIndexFuture =
+					    removeTr->getRange(prefixRange(ManagementAPI::getTenantGroupIndexKey(
+					                           finalEntry.get().tenantGroup.get(), Optional<TenantNameRef>())),
+					                       1);
+				}
+
 				state Optional<DataClusterMetadata> finalClusterMetadata =
 				    wait(tryGetClusterTransaction(removeTr, finalEntry.get().assignedCluster.get()));
 
-				if (finalClusterMetadata.present()) {
-					DataClusterEntry updatedEntry = finalClusterMetadata.get().entry;
-					// TODO: this should actually track the groups, not the tenants
+				state DataClusterEntry updatedEntry = finalClusterMetadata.get().entry;
+				state bool decrementTenantGroupCount =
+				    finalClusterMetadata.present() && !finalEntry.get().tenantGroup.present();
+
+				if (finalClusterMetadata.present() && finalEntry.get().tenantGroup.present()) {
+					RangeResult result = wait(safeThreadFutureToFuture(tenantGroupIndexFuture));
+					if (result.size() == 0) {
+						removeTr->clear(tenantGroupMetadataKeys.begin.withSuffix(finalEntry.get().tenantGroup.get()));
+						decrementTenantGroupCount = true;
+					}
+				}
+				if (decrementTenantGroupCount) {
 					--updatedEntry.allocated.numTenantGroups;
 					updateClusterMetadata(removeTr,
 					                      finalEntry.get().assignedCluster.get(),
@@ -778,9 +816,6 @@ Future<Void> deleteTenant(Reference<DB> db, TenantName name) {
 				}
 			}
 
-			wait(ManagementAPI::deleteTenantTransaction(
-			    removeTr, name, ManagementAPI::TenantOperationType::MANAGEMENT_CLUSTER));
-
 			wait(safeThreadFutureToFuture(removeTr->commit()));
 
 			break;
@@ -791,7 +826,6 @@ Future<Void> deleteTenant(Reference<DB> db, TenantName name) {
 
 	return Void();
 }
-
 }; // namespace MetaclusterAPI
 
 #include "flow/unactorcompiler.h"

fdbclient/SystemData.cpp

@@ -1420,6 +1420,8 @@ const KeyRef tenantMapPrefix = tenantMapKeys.begin;
 const KeyRef tenantMapPrivatePrefix = "\xff\xff/tenantMap/"_sr;
 const KeyRef tenantLastIdKey = "\xff/tenantLastId/"_sr;
 const KeyRef tenantDataPrefixKey = "\xff/tenantDataPrefix"_sr;
+const KeyRangeRef tenantGroupTenantIndexKeys("\xff/tenant/tenantGroup/tenantMap/"_sr,
+                                             "\xff/tenant/tenantGroup/tenantMap0"_sr);
 
 // Metacluster management cluster keys
 const KeyRangeRef dataClusterMetadataKeys("\xff/metacluster/dataCluster/metadata/"_sr,
@@ -1429,6 +1431,9 @@ const KeyRangeRef dataClusterConnectionRecordKeys("\xff/metacluster/dataCluster/
                                                   "\xff/metacluster/dataCluster/connectionString0"_sr);
 const KeyRef dataClusterConnectionRecordPrefix = dataClusterConnectionRecordKeys.begin;
 
+const KeyRangeRef tenantGroupMetadataKeys("\xff/metacluster/tenantGroup/metadata/"_sr,
+                                          "\xff/metacluster/tenantGroup/metadata0"_sr);
+
 // Metacluster data cluster keys
 const KeyRef dataClusterRegistrationKey = "\xff/metacluster/dataCluster/clusterRegistration"_sr;
 

fdbclient/SystemData.h

@@ -626,6 +626,7 @@ extern const KeyRef tenantMapPrefix;
 extern const KeyRef tenantMapPrivatePrefix;
 extern const KeyRef tenantLastIdKey;
 extern const KeyRef tenantDataPrefixKey;
+extern const KeyRangeRef tenantGroupTenantIndexKeys;
 
 Value encodeTenantEntry(TenantMapEntry const& tenantEntry);
 TenantMapEntry decodeTenantEntry(ValueRef const& value);
@@ -636,6 +637,7 @@ extern const KeyRef dataClusterMetadataPrefix;
 extern const KeyRangeRef dataClusterConnectionRecordKeys;
 extern const KeyRef dataClusterConnectionRecordPrefix;
 extern const KeyRef dataClusterRegistrationKey;
+extern const KeyRangeRef tenantGroupMetadataKeys;
 
 #pragma clang diagnostic pop
 

fdbclient/Tenant.h

@@ -80,11 +80,11 @@ struct TenantMapEntry {
 	}
 
 	Arena arena;
-	int64_t id;
+	int64_t id = -1;
 	Key prefix;
 	Optional<TenantGroupName> tenantGroup;
-	TenantState tenantState;
-	// TODO: fix this
+	TenantState tenantState = TenantState::READY;
+	// TODO: fix this type
 	Optional<Standalone<StringRef>> assignedCluster;
 
 	constexpr static int ROOT_PREFIX_SIZE = sizeof(id);
@@ -100,7 +100,7 @@ struct TenantMapEntry {
 		memcpy(data + subspace.size(), &swapped, 8);
 	}
 
-	TenantMapEntry() : id(-1) {}
+	TenantMapEntry() {}
 	TenantMapEntry(int64_t id, KeyRef subspace, TenantState tenantState) : id(id), tenantState(tenantState) {
 		setSubspace(subspace);
 	}
@@ -116,6 +116,7 @@ struct TenantMapEntry {
 		KeyRef subspace;
 		if (ar.isDeserializing) {
 			serializer(ar, id, subspace, tenantGroup, tenantState, assignedCluster);
+			ASSERT(tenantState >= TenantState::REGISTERING && tenantState <= TenantState::ERROR);
 			if (id >= 0) {
 				setSubspace(subspace);
 			}
@@ -124,6 +125,7 @@ struct TenantMapEntry {
 			if (!prefix.empty()) {
 				subspace = prefix.substr(0, prefix.size() - 8);
 			}
+			ASSERT(tenantState >= TenantState::REGISTERING && tenantState <= TenantState::ERROR);
 			serializer(ar, id, subspace, tenantGroup, tenantState, assignedCluster);
 		}
 	}

fdbclient/TenantManagement.actor.cpp

@@ -20,8 +20,9 @@
 
 #include <string>
 #include <map>
-#include "fdbclient/TenantManagement.actor.h"
 #include "fdbclient/SystemData.h"
+#include "fdbclient/TenantManagement.actor.h"
+#include "fdbclient/Tuple.h"
 #include "flow/actorcompiler.h" // has to be last include
 
 namespace ManagementAPI {
@@ -44,4 +45,13 @@ bool checkTenantMode(Optional<Value> tenantModeValue, bool isDataCluster, Tenant
 	return true;
 }
 
+Key getTenantGroupIndexKey(TenantGroupNameRef tenantGroup, Optional<TenantNameRef> tenant) {
+	Tuple tuple;
+	tuple.append(tenantGroup);
+	if (tenant.present()) {
+		tuple.append(tenant.get());
+	}
+	return tenantGroupTenantIndexKeys.begin.withSuffix(tuple.pack());
+}
+
 } // namespace ManagementAPI

fdbclient/TenantManagement.actor.h

@@ -84,6 +84,8 @@ Future<TenantMapEntry> getTenant(Reference<DB> db, TenantName name) {
 
 bool checkTenantMode(Optional<Value> tenantModeValue, bool isDataCluster, TenantOperationType operationType);
 
+Key getTenantGroupIndexKey(TenantGroupNameRef tenantGroup, Optional<TenantNameRef> tenant);
+
 // Creates a tenant with the given name. If the tenant already exists, an empty optional will be returned.
 ACTOR template <class Transaction>
 Future<std::pair<TenantMapEntry, bool>> createTenantTransaction(
@@ -162,6 +164,10 @@ Future<std::pair<TenantMapEntry, bool>> createTenantTransaction(
 		if (!contents.empty()) {
 			throw tenant_prefix_allocator_conflict();
 		}
+
+		tenantEntry.tenantState = TenantState::READY;
+	} else {
+		tenantEntry.tenantState = TenantState::REGISTERING;
 	}
 
 	// We don't store some metadata in the tenant entries on data clusters
@@ -170,6 +176,9 @@ Future<std::pair<TenantMapEntry, bool>> createTenantTransaction(
 	}
 
 	tr->set(tenantMapKey, encodeTenantEntry(tenantEntry));
+	if (tenantEntry.tenantGroup.present()) {
+		tr->set(getTenantGroupIndexKey(tenantEntry.tenantGroup.get(), name), ""_sr);
+	}
 	return std::make_pair(tenantEntry, true);
 }
 
@@ -264,6 +273,9 @@ Future<Void> deleteTenantTransaction(Transaction tr,
 	}
 
 	tr->clear(tenantMapKey);
+	if (tenantEntry.get().tenantGroup.present()) {
+		tr->clear(getTenantGroupIndexKey(tenantEntry.get().tenantGroup.get(), name));
+	}
 
 	return Void();
 }

flow/Arena.h

@@ -625,7 +625,9 @@ public:
 
 	// Copies string contents to dst and returns a pointer to the next byte after
 	uint8_t* copyTo(uint8_t* dst) const {
-		memcpy(dst, data, length);
+		if (length > 0) {
+			memcpy(dst, data, length);
+		}
 		return dst + length;
 	}