From fd13bc04c8da3fa4e04f46e187abf0f80cf8d6bb Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Mon, 23 Jan 2023 14:09:12 -0800 Subject: [PATCH] Update the tenant maps to be keyed by ID --- fdbcli/TenantCommands.actor.cpp | 7 +- fdbclient/BackupAgentBase.actor.cpp | 6 +- fdbclient/Metacluster.cpp | 1 + fdbclient/Tenant.cpp | 12 +- .../include/fdbclient/BackupAgent.actor.h | 2 +- .../fdbclient/MetaclusterManagement.actor.h | 519 ++++++++---------- fdbclient/include/fdbclient/Tenant.h | 46 +- .../fdbclient/TenantEntryCache.actor.h | 59 +- .../fdbclient/TenantManagement.actor.h | 246 +++++---- .../fdbclient/TenantSpecialKeys.actor.h | 14 +- fdbserver/ApplyMetadataMutation.cpp | 35 +- fdbserver/BlobGranuleServerCommon.actor.cpp | 6 +- fdbserver/BlobManager.actor.cpp | 5 +- fdbserver/BlobWorker.actor.cpp | 12 +- fdbserver/TenantCache.actor.cpp | 9 +- .../fdbserver/BlobGranuleServerCommon.actor.h | 5 +- .../include/fdbserver/ProxyCommitData.actor.h | 6 +- .../workloads/MetaclusterConsistency.actor.h | 50 +- .../workloads/TenantConsistency.actor.h | 79 ++- fdbserver/storageserver.actor.cpp | 6 +- .../BlobGranuleCorrectnessWorkload.actor.cpp | 13 +- .../MetaclusterManagementWorkload.actor.cpp | 17 +- .../TenantEntryCacheWorkload.actor.cpp | 89 ++- ...antManagementConcurrencyWorkload.actor.cpp | 8 +- .../TenantManagementWorkload.actor.cpp | 48 +- 25 files changed, 636 insertions(+), 664 deletions(-) diff --git a/fdbcli/TenantCommands.actor.cpp b/fdbcli/TenantCommands.actor.cpp index 334064fb48..2541486580 100644 --- a/fdbcli/TenantCommands.actor.cpp +++ b/fdbcli/TenantCommands.actor.cpp @@ -186,10 +186,15 @@ ACTOR Future tenantCreateCommand(Reference db, std::vector decodeBackupLogValue(Arena* arena, Version version, Reference> key_version, Database cx, - std::unordered_map* tenantMap, + std::map* tenantMap, bool provisionalProxy) { try { state uint64_t offset(0); @@ -657,7 +657,7 @@ ACTOR Future kvMutationLogToTransactions(Database cx, PromiseStream> addActor, FlowLock* commitLock, Reference> keyVersion, - std::unordered_map* tenantMap, + std::map* tenantMap, bool provisionalProxy) { state Version lastVersion = invalidVersion; state bool endOfStream = false; @@ -789,7 +789,7 @@ ACTOR Future applyMutations(Database cx, PublicRequestStream commit, NotifiedVersion* committedVersion, Reference> keyVersion, - std::unordered_map* tenantMap, + std::map* tenantMap, bool provisionalProxy) { state FlowLock commitLock(CLIENT_KNOBS->BACKUP_LOCK_BYTES); state PromiseStream> addActor; diff --git a/fdbclient/Metacluster.cpp b/fdbclient/Metacluster.cpp index 993b70fa70..b298762f11 100644 --- a/fdbclient/Metacluster.cpp +++ b/fdbclient/Metacluster.cpp @@ -23,6 +23,7 @@ FDB_DEFINE_BOOLEAN_PARAM(AddNewTenants); FDB_DEFINE_BOOLEAN_PARAM(RemoveMissingTenants); +FDB_DEFINE_BOOLEAN_PARAM(AssignClusterAutomatically); std::string clusterTypeToString(const ClusterType& clusterType) { switch (clusterType) { diff --git a/fdbclient/Tenant.cpp b/fdbclient/Tenant.cpp index 0ef1775494..002facf9c0 100644 --- a/fdbclient/Tenant.cpp +++ b/fdbclient/Tenant.cpp @@ -60,10 +60,8 @@ std::string TenantMapEntry::tenantStateToString(TenantState tenantState) { return "removing"; case TenantState::UPDATING_CONFIGURATION: return "updating configuration"; - case TenantState::RENAMING_FROM: - return "renaming from"; - case TenantState::RENAMING_TO: - return "renaming to"; + case TenantState::RENAMING: + return "renaming"; case TenantState::ERROR: return "error"; default: @@ -81,10 +79,8 @@ TenantState TenantMapEntry::stringToTenantState(std::string stateStr) { return TenantState::REMOVING; } else if (stateStr == "updating configuration") { return TenantState::UPDATING_CONFIGURATION; - } else if (stateStr == "renaming from") { - return TenantState::RENAMING_FROM; - } else if (stateStr == "renaming to") { - return TenantState::RENAMING_TO; + } else if (stateStr == "renaming") { + return TenantState::RENAMING; } else if (stateStr == "error") { return TenantState::ERROR; } diff --git a/fdbclient/include/fdbclient/BackupAgent.actor.h b/fdbclient/include/fdbclient/BackupAgent.actor.h index 266e30e82c..c7e1e666be 100644 --- a/fdbclient/include/fdbclient/BackupAgent.actor.h +++ b/fdbclient/include/fdbclient/BackupAgent.actor.h @@ -575,7 +575,7 @@ ACTOR Future applyMutations(Database cx, PublicRequestStream commit, NotifiedVersion* committedVersion, Reference> keyVersion, - std::unordered_map* tenantMap, + std::map* tenantMap, bool provisionalProxy); ACTOR Future cleanupBackup(Database cx, DeleteData deleteData); diff --git a/fdbclient/include/fdbclient/MetaclusterManagement.actor.h b/fdbclient/include/fdbclient/MetaclusterManagement.actor.h index 63e439b159..c745916f9a 100644 --- a/fdbclient/include/fdbclient/MetaclusterManagement.actor.h +++ b/fdbclient/include/fdbclient/MetaclusterManagement.actor.h @@ -20,7 +20,9 @@ #pragma once #include "fdbclient/FDBOptions.g.h" +#include "fdbclient/Tenant.h" #include "flow/IRandom.h" +#include "flow/Platform.h" #include "flow/ThreadHelper.actor.h" #if defined(NO_INTELLISENSE) && !defined(FDBCLIENT_METACLUSTER_MANAGEMENT_ACTOR_G_H) #define FDBCLIENT_METACLUSTER_MANAGEMENT_ACTOR_G_H @@ -80,6 +82,7 @@ struct DataClusterMetadata { FDB_DECLARE_BOOLEAN_PARAM(AddNewTenants); FDB_DECLARE_BOOLEAN_PARAM(RemoveMissingTenants); +FDB_DECLARE_BOOLEAN_PARAM(AssignClusterAutomatically); namespace MetaclusterAPI { @@ -373,20 +376,33 @@ struct MetaclusterOperationContext { }; template -Future> tryGetTenantTransaction(Transaction tr, TenantName name) { +Future> tryGetTenantTransaction(Transaction tr, int64_t tenantId) { tr->setOption(FDBTransactionOptions::RAW_ACCESS); - return ManagementClusterMetadata::tenantMetadata().tenantMap.get(tr, name); + return ManagementClusterMetadata::tenantMetadata().tenantMap.get(tr, tenantId); } -ACTOR template -Future> tryGetTenant(Reference db, TenantName name) { +ACTOR template +Future> tryGetTenantTransaction(Transaction tr, TenantName name) { + tr->setOption(FDBTransactionOptions::RAW_ACCESS); + Optional tenantId = wait(ManagementClusterMetadata::tenantMetadata().tenantNameIndex.get(tr, name)); + if (tenantId.present()) { + Optional entry = + wait(ManagementClusterMetadata::tenantMetadata().tenantMap.get(tr, tenantId.get())); + return entry; + } else { + return Optional(); + } +} + +ACTOR template +Future> tryGetTenant(Reference db, Tenant tenant) { state Reference tr = db->createTransaction(); loop { try { tr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS); tr->setOption(FDBTransactionOptions::READ_LOCK_AWARE); - Optional entry = wait(tryGetTenantTransaction(tr, name)); + Optional entry = wait(tryGetTenantTransaction(tr, tenant)); return entry; } catch (Error& e) { wait(safeThreadFutureToFuture(tr->onError(e))); @@ -394,9 +410,9 @@ Future> tryGetTenant(Reference db, TenantName name) } } -ACTOR template -Future getTenantTransaction(Transaction tr, TenantName name) { - Optional entry = wait(tryGetTenantTransaction(tr, name)); +ACTOR template +Future getTenantTransaction(Transaction tr, Tenant tenant) { + Optional entry = wait(tryGetTenantTransaction(tr, tenant)); if (!entry.present()) { throw tenant_not_found(); } @@ -404,9 +420,9 @@ Future getTenantTransaction(Transaction tr, TenantName name) { return entry.get(); } -ACTOR template -Future getTenant(Reference db, TenantName name) { - Optional entry = wait(tryGetTenant(db, name)); +ACTOR template +Future getTenant(Reference db, Tenant tenant) { + Optional entry = wait(tryGetTenant(db, tenant)); if (!entry.present()) { throw tenant_not_found(); } @@ -416,12 +432,12 @@ Future getTenant(Reference db, TenantName name) { ACTOR template Future managementClusterCheckEmpty(Transaction tr) { - state Future>> tenantsFuture = + state Future>> tenantsFuture = TenantMetadata::tenantMap().getRange(tr, {}, {}, 1); state typename transaction_future_type::type dbContentsFuture = tr->getRange(normalKeys, 1); - KeyBackedRangeResult> tenants = wait(tenantsFuture); + KeyBackedRangeResult> tenants = wait(tenantsFuture); if (!tenants.results.empty()) { throw cluster_not_empty(); } @@ -796,8 +812,8 @@ struct RemoveClusterImpl { // Erase each tenant from the tenant map on the management cluster for (Tuple entry : tenantEntries.results) { ASSERT(entry.getString(0) == self->ctx.clusterName.get()); - ManagementClusterMetadata::tenantMetadata().tenantMap.erase(tr, entry.getString(1)); - ManagementClusterMetadata::tenantMetadata().tenantIdIndex.erase(tr, entry.getInt(2)); + ManagementClusterMetadata::tenantMetadata().tenantMap.erase(tr, entry.getInt(2)); + ManagementClusterMetadata::tenantMetadata().tenantNameIndex.erase(tr, entry.getString(1)); ManagementClusterMetadata::tenantMetadata().lastTenantModification.setVersionstamp(tr, Versionstamp(), 0); } @@ -994,7 +1010,6 @@ Future> listClusters(Reference db template void managementClusterAddTenantToGroup(Transaction tr, - TenantName tenantName, TenantMapEntry tenantEntry, DataClusterMetadata* clusterMetadata, bool groupAlreadyExists) { @@ -1010,7 +1025,7 @@ void managementClusterAddTenantToGroup(Transaction tr, tr, Tuple::makeTuple(tenantEntry.assignedCluster.get(), tenantEntry.tenantGroup.get())); } ManagementClusterMetadata::tenantMetadata().tenantGroupTenantIndex.insert( - tr, Tuple::makeTuple(tenantEntry.tenantGroup.get(), tenantName)); + tr, Tuple::makeTuple(tenantEntry.tenantGroup.get(), tenantEntry.id)); } if (!groupAlreadyExists) { @@ -1028,14 +1043,12 @@ void managementClusterAddTenantToGroup(Transaction tr, ACTOR template Future managementClusterRemoveTenantFromGroup(Transaction tr, - TenantName tenantName, TenantMapEntry tenantEntry, - DataClusterMetadata* clusterMetadata, - bool isRenamePair = false) { - state bool updateClusterCapacity = !tenantEntry.tenantGroup.present() && !isRenamePair; + DataClusterMetadata* clusterMetadata) { + state bool updateClusterCapacity = !tenantEntry.tenantGroup.present(); if (tenantEntry.tenantGroup.present()) { ManagementClusterMetadata::tenantMetadata().tenantGroupTenantIndex.erase( - tr, Tuple::makeTuple(tenantEntry.tenantGroup.get(), tenantName)); + tr, Tuple::makeTuple(tenantEntry.tenantGroup.get(), tenantEntry.id)); state KeyBackedSet::RangeResultType result = wait(ManagementClusterMetadata::tenantMetadata().tenantGroupTenantIndex.getRange( @@ -1070,21 +1083,18 @@ Future managementClusterRemoveTenantFromGroup(Transaction tr, template struct CreateTenantImpl { MetaclusterOperationContext ctx; - bool preferAssignedCluster; + AssignClusterAutomatically assignClusterAutomatically; // Initialization parameters - TenantName tenantName; TenantMapEntry tenantEntry; // Parameter set if tenant creation permanently fails on the data cluster Optional replaceExistingTenantId; CreateTenantImpl(Reference managementDb, - bool preferAssignedCluster, - TenantName tenantName, - TenantMapEntry tenantEntry) - : ctx(managementDb), preferAssignedCluster(preferAssignedCluster), tenantName(tenantName), - tenantEntry(tenantEntry) {} + TenantMapEntry tenantEntry, + AssignClusterAutomatically assignClusterAutomatically) + : ctx(managementDb), tenantEntry(tenantEntry), assignClusterAutomatically(assignClusterAutomatically) {} ACTOR static Future checkClusterAvailability(Reference dataClusterDb, ClusterName clusterName) { @@ -1107,7 +1117,7 @@ struct CreateTenantImpl { ACTOR static Future checkForExistingTenant(CreateTenantImpl* self, Reference tr) { // Check if the tenant already exists. If it's partially created and matches the parameters we // specified, continue creating it. Otherwise, fail with an error. - state Optional existingEntry = wait(tryGetTenantTransaction(tr, self->tenantName)); + state Optional existingEntry = wait(tryGetTenantTransaction(tr, self->tenantEntry.tenantName)); if (existingEntry.present()) { if (!existingEntry.get().matchesConfiguration(self->tenantEntry) || existingEntry.get().tenantState != TenantState::REGISTERING) { @@ -1117,12 +1127,11 @@ struct CreateTenantImpl { } else if (!self->replaceExistingTenantId.present() || self->replaceExistingTenantId.get() != existingEntry.get().id) { // The tenant creation has already started, so resume where we left off - ASSERT(existingEntry.get().assignedCluster.present()); - if (self->preferAssignedCluster && - existingEntry.get().assignedCluster.get() != self->tenantEntry.assignedCluster.get()) { + if (!self->assignClusterAutomatically && + existingEntry.get().assignedCluster != self->tenantEntry.assignedCluster) { TraceEvent("MetaclusterCreateTenantClusterMismatch") - .detail("Preferred", self->tenantEntry.assignedCluster.get()) - .detail("Actual", existingEntry.get().assignedCluster.get()); + .detail("Preferred", self->tenantEntry.assignedCluster) + .detail("Actual", existingEntry.get().assignedCluster); throw invalid_tenant_configuration(); } self->tenantEntry = existingEntry.get(); @@ -1130,23 +1139,23 @@ struct CreateTenantImpl { return true; } else { // The previous creation is permanently failed, so cleanup the tenant and create it again from scratch - // We don't need to remove it from the tenant map because we will overwrite the existing entry later in - // this transaction. - ManagementClusterMetadata::tenantMetadata().tenantIdIndex.erase(tr, existingEntry.get().id); + // We don't need to remove it from the tenantNameIndex because we will overwrite the existing entry + // later in this transaction. + ManagementClusterMetadata::tenantMetadata().tenantMap.erase(tr, existingEntry.get().id); ManagementClusterMetadata::tenantMetadata().tenantCount.atomicOp(tr, -1, MutationRef::AddValue); ManagementClusterMetadata::clusterTenantCount.atomicOp( tr, existingEntry.get().assignedCluster.get(), -1, MutationRef::AddValue); ManagementClusterMetadata::clusterTenantIndex.erase( tr, - Tuple::makeTuple( - existingEntry.get().assignedCluster.get(), self->tenantName, existingEntry.get().id)); + Tuple::makeTuple(existingEntry.get().assignedCluster.get(), + self->tenantEntry.tenantName, + existingEntry.get().id)); state DataClusterMetadata previousAssignedClusterMetadata = wait(getClusterTransaction(tr, existingEntry.get().assignedCluster.get())); - wait(managementClusterRemoveTenantFromGroup( - tr, self->tenantName, existingEntry.get(), &previousAssignedClusterMetadata)); + wait(managementClusterRemoveTenantFromGroup(tr, existingEntry.get(), &previousAssignedClusterMetadata)); } } else if (self->replaceExistingTenantId.present()) { throw tenant_removed(); @@ -1168,7 +1177,7 @@ struct CreateTenantImpl { if (groupEntry.present()) { ASSERT(groupEntry.get().assignedCluster.present()); - if (self->preferAssignedCluster && + if (!self->assignClusterAutomatically && groupEntry.get().assignedCluster.get() != self->tenantEntry.assignedCluster.get()) { TraceEvent("MetaclusterCreateTenantGroupClusterMismatch") .detail("TenantGroupCluster", groupEntry.get().assignedCluster.get()) @@ -1184,7 +1193,7 @@ struct CreateTenantImpl { state std::vector> clusterAvailabilityChecks; // Get a set of the most full clusters that still have capacity // If preferred cluster is specified, look for that one. - if (self->preferAssignedCluster) { + if (!self->assignClusterAutomatically) { DataClusterMetadata dataClusterMetadata = wait(getClusterTransaction(tr, self->tenantEntry.assignedCluster.get())); if (!dataClusterMetadata.entry.hasCapacity()) { @@ -1262,8 +1271,9 @@ struct CreateTenantImpl { ManagementClusterMetadata::tenantMetadata().lastTenantId.set(tr, self->tenantEntry.id); self->tenantEntry.tenantState = TenantState::REGISTERING; - ManagementClusterMetadata::tenantMetadata().tenantMap.set(tr, self->tenantName, self->tenantEntry); - ManagementClusterMetadata::tenantMetadata().tenantIdIndex.set(tr, self->tenantEntry.id, self->tenantName); + ManagementClusterMetadata::tenantMetadata().tenantMap.set(tr, self->tenantEntry.id, self->tenantEntry); + ManagementClusterMetadata::tenantMetadata().tenantNameIndex.set( + tr, self->tenantEntry.tenantName, self->tenantEntry.id); ManagementClusterMetadata::tenantMetadata().lastTenantModification.setVersionstamp(tr, Versionstamp(), 0); ManagementClusterMetadata::tenantMetadata().tenantCount.atomicOp(tr, 1, MutationRef::AddValue); @@ -1278,8 +1288,10 @@ struct CreateTenantImpl { } // Updated indexes to include the new tenant - ManagementClusterMetadata::clusterTenantIndex.insert( - tr, Tuple::makeTuple(self->tenantEntry.assignedCluster.get(), self->tenantName, self->tenantEntry.id)); + ManagementClusterMetadata::clusterTenantIndex.insert(tr, + Tuple::makeTuple(self->tenantEntry.assignedCluster.get(), + self->tenantEntry.tenantName, + self->tenantEntry.id)); wait(setClusterFuture); @@ -1290,14 +1302,14 @@ struct CreateTenantImpl { } managementClusterAddTenantToGroup( - tr, self->tenantName, self->tenantEntry, &self->ctx.dataClusterMetadata.get(), assignment.second); + tr, self->tenantEntry, &self->ctx.dataClusterMetadata.get(), assignment.second); return Void(); } ACTOR static Future storeTenantInDataCluster(CreateTenantImpl* self, Reference tr) { - std::pair, bool> dataClusterTenant = wait( - TenantAPI::createTenantTransaction(tr, self->tenantName, self->tenantEntry, ClusterType::METACLUSTER_DATA)); + std::pair, bool> dataClusterTenant = + wait(TenantAPI::createTenantTransaction(tr, self->tenantEntry, ClusterType::METACLUSTER_DATA)); // If the tenant map entry is empty, then we encountered a tombstone indicating that the tenant was // simultaneously removed. @@ -1309,17 +1321,15 @@ struct CreateTenantImpl { } ACTOR static Future markTenantReady(CreateTenantImpl* self, Reference tr) { - state Optional managementEntry = wait(tryGetTenantTransaction(tr, self->tenantName)); + state Optional managementEntry = wait(tryGetTenantTransaction(tr, self->tenantEntry.id)); if (!managementEntry.present()) { throw tenant_removed(); - } else if (managementEntry.get().id != self->tenantEntry.id) { - throw tenant_already_exists(); } if (managementEntry.get().tenantState == TenantState::REGISTERING) { TenantMapEntry updatedEntry = managementEntry.get(); updatedEntry.tenantState = TenantState::READY; - ManagementClusterMetadata::tenantMetadata().tenantMap.set(tr, self->tenantName, updatedEntry); + ManagementClusterMetadata::tenantMetadata().tenantMap.set(tr, updatedEntry.id, updatedEntry); ManagementClusterMetadata::tenantMetadata().lastTenantModification.setVersionstamp(tr, Versionstamp(), 0); } @@ -1327,7 +1337,7 @@ struct CreateTenantImpl { } ACTOR static Future run(CreateTenantImpl* self) { - if (self->tenantName.startsWith("\xff"_sr)) { + if (self->tenantEntry.tenantName.startsWith("\xff"_sr)) { throw invalid_tenant_name(); } @@ -1361,8 +1371,10 @@ struct CreateTenantImpl { }; ACTOR template -Future createTenant(Reference db, TenantName name, TenantMapEntry tenantEntry) { - state CreateTenantImpl impl(db, tenantEntry.assignedCluster.present(), name, tenantEntry); +Future createTenant(Reference db, + TenantMapEntry tenantEntry, + AssignClusterAutomatically assignClusterAutomatically) { + state CreateTenantImpl impl(db, tenantEntry, assignClusterAutomatically); wait(impl.run()); return Void(); } @@ -1374,44 +1386,35 @@ struct DeleteTenantImpl { // Initialization parameters // Either one can be specified, and the other will be looked up // and filled in by reading the metacluster metadata - TenantName tenantName; + Optional tenantName; int64_t tenantId = -1; - // Parameters set in markTenantInRemovingState - Optional pairName; - DeleteTenantImpl(Reference managementDb, TenantName tenantName) : ctx(managementDb), tenantName(tenantName) {} DeleteTenantImpl(Reference managementDb, int64_t tenantId) : ctx(managementDb), tenantId(tenantId) {} // Loads the cluster details for the cluster where the tenant is assigned. // Returns true if the deletion is already in progress - ACTOR static Future getAssignedLocation(DeleteTenantImpl* self, Reference tr) { - // Look at tenantIdIndex if given ID, then fill out the corresponding name - if (self->tenantId != -1) { - TenantName indexName = - wait(ManagementClusterMetadata::tenantMetadata().tenantIdIndex.getD(tr, self->tenantId)); - self->tenantName = indexName; + ACTOR static Future> getAssignedLocation(DeleteTenantImpl* self, + Reference tr) { + state int64_t resolvedId = self->tenantId; + if (self->tenantId == -1) { + ASSERT(self->tenantName.present()); + wait(store(resolvedId, + ManagementClusterMetadata::tenantMetadata().tenantNameIndex.getD( + tr, self->tenantName.get(), Snapshot::False, TenantInfo::INVALID_TENANT))); } - state Optional tenantEntry = wait(tryGetTenantTransaction(tr, self->tenantName)); - if (!tenantEntry.present()) { - throw tenant_not_found(); - } + state TenantMapEntry tenantEntry = wait(getTenantTransaction(tr, resolvedId)); // Disallow removing the "new" name of a renamed tenant before it completes - if (tenantEntry.get().tenantState == TenantState::RENAMING_TO) { + if (self->tenantName.present() && tenantEntry.tenantName != self->tenantName.get()) { + ASSERT(tenantEntry.tenantState == TenantState::RENAMING || + tenantEntry.tenantState == TenantState::REMOVING); throw tenant_not_found(); } - if (tenantEntry.get().tenantState == TenantState::REMOVING) { - if (tenantEntry.get().renamePair.present()) { - self->pairName = tenantEntry.get().renamePair.get(); - } - } - ASSERT(self->tenantId == -1 || self->tenantId == tenantEntry.get().id); - self->tenantId = tenantEntry.get().id; - wait(self->ctx.setCluster(tr, tenantEntry.get().assignedCluster.get())); - return tenantEntry.get().tenantState == TenantState::REMOVING; + wait(self->ctx.setCluster(tr, tenantEntry.assignedCluster.get())); + return std::make_pair(resolvedId, tenantEntry.tenantState == TenantState::REMOVING); } // Does an initial check if the tenant is empty. This is an optimization to prevent us marking a tenant @@ -1420,8 +1423,8 @@ struct DeleteTenantImpl { // // SOMEDAY: should this also lock the tenant when locking is supported? ACTOR static Future checkTenantEmpty(DeleteTenantImpl* self, Reference tr) { - state Optional tenantEntry = wait(TenantAPI::tryGetTenantTransaction(tr, self->tenantName)); - if (!tenantEntry.present() || tenantEntry.get().id != self->tenantId) { + state Optional tenantEntry = wait(TenantAPI::tryGetTenantTransaction(tr, self->tenantId)); + if (!tenantEntry.present()) { // The tenant must have been removed simultaneously return Void(); } @@ -1438,41 +1441,13 @@ struct DeleteTenantImpl { // Mark the tenant as being in a removing state on the management cluster ACTOR static Future markTenantInRemovingState(DeleteTenantImpl* self, Reference tr) { - state Optional tenantEntry = wait(tryGetTenantTransaction(tr, self->tenantName)); + state TenantMapEntry tenantEntry = wait(getTenantTransaction(tr, self->tenantId)); - if (!tenantEntry.present() || tenantEntry.get().id != self->tenantId || - tenantEntry.get().tenantState == TenantState::RENAMING_TO) { - throw tenant_not_found(); - } + if (tenantEntry.tenantState != TenantState::REMOVING) { + tenantEntry.tenantState = TenantState::REMOVING; - if (tenantEntry.get().renamePair.present()) { - ASSERT(tenantEntry.get().tenantState == TenantState::RENAMING_FROM || - tenantEntry.get().tenantState == TenantState::REMOVING); - - self->pairName = tenantEntry.get().renamePair.get(); - } - - if (tenantEntry.get().tenantState != TenantState::REMOVING) { - state TenantMapEntry updatedEntry = tenantEntry.get(); - // Check if we are deleting a tenant in the middle of a rename - updatedEntry.tenantState = TenantState::REMOVING; - ManagementClusterMetadata::tenantMetadata().tenantMap.set(tr, self->tenantName, updatedEntry); + ManagementClusterMetadata::tenantMetadata().tenantMap.set(tr, tenantEntry.id, tenantEntry); ManagementClusterMetadata::tenantMetadata().lastTenantModification.setVersionstamp(tr, Versionstamp(), 0); - - // If this has a rename pair, also mark the other entry for deletion - if (self->pairName.present()) { - state Optional pairEntry = wait(tryGetTenantTransaction(tr, self->pairName.get())); - TenantMapEntry updatedPairEntry = pairEntry.get(); - // Sanity check that our pair has us named as their partner - ASSERT(updatedPairEntry.renamePair.present()); - ASSERT(updatedPairEntry.renamePair.get() == self->tenantName); - ASSERT(updatedPairEntry.id == self->tenantId); - CODE_PROBE(true, "marking pair tenant in removing state"); - updatedPairEntry.tenantState = TenantState::REMOVING; - ManagementClusterMetadata::tenantMetadata().tenantMap.set(tr, self->pairName.get(), updatedPairEntry); - ManagementClusterMetadata::tenantMetadata().lastTenantModification.setVersionstamp( - tr, Versionstamp(), 0); - } } return Void(); @@ -1480,27 +1455,18 @@ struct DeleteTenantImpl { // Delete the tenant and related metadata on the management cluster ACTOR static Future deleteTenantFromManagementCluster(DeleteTenantImpl* self, - Reference tr, - bool pairDelete = false) { - // If pair is present, and this is not already a pair delete, call this function recursively - state Future pairFuture = Void(); - if (!pairDelete && self->pairName.present()) { - CODE_PROBE(true, "deleting pair tenant from management cluster"); - pairFuture = deleteTenantFromManagementCluster(self, tr, true); - } - state TenantName tenantName = pairDelete ? self->pairName.get() : self->tenantName; - state Optional tenantEntry = wait(tryGetTenantTransaction(tr, tenantName)); + Reference tr) { + state Optional tenantEntry = wait(tryGetTenantTransaction(tr, self->tenantId)); - if (!tenantEntry.present() || tenantEntry.get().id != self->tenantId) { + if (!tenantEntry.present()) { return Void(); } - ASSERT(tenantEntry.get().tenantState == TenantState::REMOVING && - (pairDelete || tenantEntry.get().renamePair == self->pairName)); + ASSERT(tenantEntry.get().tenantState == TenantState::REMOVING); // Erase the tenant entry itself - ManagementClusterMetadata::tenantMetadata().tenantMap.erase(tr, tenantName); - ManagementClusterMetadata::tenantMetadata().tenantIdIndex.erase(tr, tenantEntry.get().id); + ManagementClusterMetadata::tenantMetadata().tenantMap.erase(tr, tenantEntry.get().id); + ManagementClusterMetadata::tenantMetadata().tenantNameIndex.erase(tr, tenantEntry.get().tenantName); ManagementClusterMetadata::tenantMetadata().lastTenantModification.setVersionstamp(tr, Versionstamp(), 0); // This is idempotent because this function is only called if the tenant is in the map @@ -1510,22 +1476,39 @@ struct DeleteTenantImpl { // Remove the tenant from the cluster -> tenant index ManagementClusterMetadata::clusterTenantIndex.erase( - tr, Tuple::makeTuple(tenantEntry.get().assignedCluster.get(), tenantName, self->tenantId)); + tr, + Tuple::makeTuple(tenantEntry.get().assignedCluster.get(), tenantEntry.get().tenantName, self->tenantId)); + + if (tenantEntry.get().renameDestination.present()) { + // If renaming, remove the metadata associated with the tenant destination + ManagementClusterMetadata::tenantMetadata().tenantNameIndex.erase( + tr, tenantEntry.get().renameDestination.get()); + + ManagementClusterMetadata::clusterTenantIndex.erase( + tr, + Tuple::makeTuple(tenantEntry.get().assignedCluster.get(), + tenantEntry.get().renameDestination.get(), + self->tenantId)); + } // Remove the tenant from its tenant group - wait(managementClusterRemoveTenantFromGroup( - tr, tenantName, tenantEntry.get(), &self->ctx.dataClusterMetadata.get(), pairDelete)); + wait(managementClusterRemoveTenantFromGroup(tr, tenantEntry.get(), &self->ctx.dataClusterMetadata.get())); - wait(pairFuture); return Void(); } ACTOR static Future run(DeleteTenantImpl* self) { // Get information about the tenant and where it is assigned - bool deletionInProgress = wait(self->ctx.runManagementTransaction( + std::pair result = wait(self->ctx.runManagementTransaction( [self = self](Reference tr) { return getAssignedLocation(self, tr); })); - if (!deletionInProgress) { + if (self->tenantId == -1) { + self->tenantId = result.first; + } else { + ASSERT(result.first == self->tenantId); + } + + if (!result.second) { wait(self->ctx.runDataClusterTransaction( [self = self](Reference tr) { return checkTenantEmpty(self, tr); })); @@ -1536,16 +1519,7 @@ struct DeleteTenantImpl { // Delete tenant on the data cluster wait(self->ctx.runDataClusterTransaction([self = self](Reference tr) { - // If the removed tenant is being renamed, attempt to delete both the old and new names. - // At most one should be present with the given ID, and the other will be a no-op. - Future pairDelete = Void(); - if (self->pairName.present()) { - CODE_PROBE(true, "deleting pair tenant from data cluster"); - pairDelete = TenantAPI::deleteTenantTransaction( - tr, self->pairName.get(), self->tenantId, ClusterType::METACLUSTER_DATA); - } - return pairDelete && TenantAPI::deleteTenantTransaction( - tr, self->tenantName, self->tenantId, ClusterType::METACLUSTER_DATA); + return TenantAPI::deleteTenantTransaction(tr, self->tenantId, ClusterType::METACLUSTER_DATA); })); wait(self->ctx.runManagementTransaction([self = self](Reference tr) { return deleteTenantFromManagementCluster(self, tr); @@ -1577,10 +1551,23 @@ Future>> listTenantsTransactio int limit) { tr->setOption(FDBTransactionOptions::RAW_ACCESS); - state KeyBackedRangeResult> results = - wait(ManagementClusterMetadata::tenantMetadata().tenantMap.getRange(tr, begin, end, limit)); + state KeyBackedRangeResult> matchingTenants = + wait(ManagementClusterMetadata::tenantMetadata().tenantNameIndex.getRange(tr, begin, end, limit)); - return results.results; + state std::vector> tenantEntryFutures; + for (auto const& [name, id] : matchingTenants.results) { + tenantEntryFutures.push_back(getTenantTransaction(tr, id)); + } + + wait(waitForAll(tenantEntryFutures)); + + std::vector> results; + for (int i = 0; i < matchingTenants.results.size(); ++i) { + // Tenants being renamed will show up twice; once under each name + results.emplace_back(matchingTenants.results[i].first, tenantEntryFutures[i].get()); + } + + return results; } ACTOR template @@ -1592,48 +1579,49 @@ Future>> listTenants( int offset = 0, std::vector filters = std::vector()) { state Reference tr = db->createTransaction(); + state std::vector> results; loop { try { tr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS); tr->setOption(FDBTransactionOptions::READ_LOCK_AWARE); if (filters.empty()) { - state std::vector> tenants; - wait(store(tenants, listTenantsTransaction(tr, begin, end, limit + offset))); - if (offset >= tenants.size()) { - tenants.clear(); + wait(store(results, listTenantsTransaction(tr, begin, end, limit + offset))); + + if (offset >= results.size()) { + results.clear(); } else if (offset > 0) { - tenants.erase(tenants.begin(), tenants.begin() + offset); + results.erase(results.begin(), results.begin() + offset); } - return tenants; + + return results; } + tr->setOption(FDBTransactionOptions::RAW_ACCESS); - state KeyBackedRangeResult> results = - wait(ManagementClusterMetadata::tenantMetadata().tenantMap.getRange( - tr, begin, end, std::max(limit + offset, 100))); - state std::vector> filterResults; state int count = 0; loop { - for (auto pair : results.results) { - if (filters.empty() || std::count(filters.begin(), filters.end(), pair.second.tenantState)) { + std::vector> tenantBatch = + wait(listTenantsTransaction(tr, begin, end, std::max(limit + offset, 1000))); + + if (tenantBatch.empty()) { + return results; + } + + for (auto const& [name, entry] : tenantBatch) { + if (filters.empty() || std::count(filters.begin(), filters.end(), entry.tenantState)) { ++count; if (count > offset) { - filterResults.push_back(pair); + results.push_back(std::make_pair(name, entry)); if (count - offset == limit) { - ASSERT(count - offset == filterResults.size()); - return filterResults; + ASSERT(count - offset == results.size()); + return results; } } } } - if (!results.more) { - return filterResults; - } - begin = keyAfter(results.results.back().first); - wait(store(results, - ManagementClusterMetadata::tenantMetadata().tenantMap.getRange( - tr, begin, end, std::max(limit + offset, 100)))); + + begin = keyAfter(tenantBatch.back().first); } } catch (Error& e) { wait(safeThreadFutureToFuture(tr->onError(e))); @@ -1677,10 +1665,8 @@ struct ConfigureTenantImpl { throw cluster_no_capacity(); } - wait(managementClusterRemoveTenantFromGroup( - tr, self->tenantName, tenantEntry, &self->ctx.dataClusterMetadata.get())); - managementClusterAddTenantToGroup( - tr, self->tenantName, entryWithUpdatedGroup, &self->ctx.dataClusterMetadata.get(), false); + wait(managementClusterRemoveTenantFromGroup(tr, tenantEntry, &self->ctx.dataClusterMetadata.get())); + managementClusterAddTenantToGroup(tr, entryWithUpdatedGroup, &self->ctx.dataClusterMetadata.get(), false); return Void(); } @@ -1692,19 +1678,15 @@ struct ConfigureTenantImpl { if (!self->ctx.dataClusterMetadata.get().entry.hasCapacity()) { throw cluster_no_capacity(); } - wait(managementClusterRemoveTenantFromGroup( - tr, self->tenantName, tenantEntry, &self->ctx.dataClusterMetadata.get())); - managementClusterAddTenantToGroup( - tr, self->tenantName, entryWithUpdatedGroup, &self->ctx.dataClusterMetadata.get(), false); + wait(managementClusterRemoveTenantFromGroup(tr, tenantEntry, &self->ctx.dataClusterMetadata.get())); + managementClusterAddTenantToGroup(tr, entryWithUpdatedGroup, &self->ctx.dataClusterMetadata.get(), false); return Void(); } // Moves between groups in the same cluster are freely allowed else if (tenantGroupEntry.get().assignedCluster == tenantEntry.assignedCluster) { - wait(managementClusterRemoveTenantFromGroup( - tr, self->tenantName, tenantEntry, &self->ctx.dataClusterMetadata.get())); - managementClusterAddTenantToGroup( - tr, self->tenantName, entryWithUpdatedGroup, &self->ctx.dataClusterMetadata.get(), true); + wait(managementClusterRemoveTenantFromGroup(tr, tenantEntry, &self->ctx.dataClusterMetadata.get())); + managementClusterAddTenantToGroup(tr, entryWithUpdatedGroup, &self->ctx.dataClusterMetadata.get(), true); return Void(); } @@ -1750,7 +1732,7 @@ struct ConfigureTenantImpl { } ++self->updatedEntry.configurationSequenceNum; - ManagementClusterMetadata::tenantMetadata().tenantMap.set(tr, self->tenantName, self->updatedEntry); + ManagementClusterMetadata::tenantMetadata().tenantMap.set(tr, self->updatedEntry.id, self->updatedEntry); ManagementClusterMetadata::tenantMetadata().lastTenantModification.setVersionstamp(tr, Versionstamp(), 0); return Void(); @@ -1758,9 +1740,10 @@ struct ConfigureTenantImpl { // Updates the configuration in the data cluster ACTOR static Future updateDataCluster(ConfigureTenantImpl* self, Reference tr) { - state Optional tenantEntry = wait(TenantAPI::tryGetTenantTransaction(tr, self->tenantName)); + state Optional tenantEntry = + wait(TenantAPI::tryGetTenantTransaction(tr, self->updatedEntry.id)); - if (!tenantEntry.present() || tenantEntry.get().id != self->updatedEntry.id || + if (!tenantEntry.present() || tenantEntry.get().configurationSequenceNum >= self->updatedEntry.configurationSequenceNum) { // If the tenant isn't in the metacluster, it must have been concurrently removed return Void(); @@ -1770,23 +1753,22 @@ struct ConfigureTenantImpl { dataClusterEntry.tenantState = TenantState::READY; dataClusterEntry.assignedCluster = {}; - wait(TenantAPI::configureTenantTransaction(tr, self->tenantName, tenantEntry.get(), dataClusterEntry)); + wait(TenantAPI::configureTenantTransaction(tr, tenantEntry.get(), dataClusterEntry)); return Void(); } // Updates the tenant state in the management cluster to READY ACTOR static Future markManagementTenantAsReady(ConfigureTenantImpl* self, Reference tr) { - state Optional tenantEntry = wait(tryGetTenantTransaction(tr, self->tenantName)); + state Optional tenantEntry = wait(tryGetTenantTransaction(tr, self->updatedEntry.id)); - if (!tenantEntry.present() || tenantEntry.get().id != self->updatedEntry.id || - tenantEntry.get().tenantState != TenantState::UPDATING_CONFIGURATION || + if (!tenantEntry.present() || tenantEntry.get().tenantState != TenantState::UPDATING_CONFIGURATION || tenantEntry.get().configurationSequenceNum > self->updatedEntry.configurationSequenceNum) { return Void(); } tenantEntry.get().tenantState = TenantState::READY; - ManagementClusterMetadata::tenantMetadata().tenantMap.set(tr, self->tenantName, tenantEntry.get()); + ManagementClusterMetadata::tenantMetadata().tenantMap.set(tr, tenantEntry.get().id, tenantEntry.get()); ManagementClusterMetadata::tenantMetadata().lastTenantModification.setVersionstamp(tr, Versionstamp(), 0); return Void(); } @@ -1828,114 +1810,77 @@ struct RenameTenantImpl { RenameTenantImpl(Reference managementDb, TenantName oldName, TenantName newName) : ctx(managementDb), oldName(oldName), newName(newName) {} - // Delete the tenant and related metadata on the management cluster - ACTOR static Future deleteTenantFromManagementCluster(RenameTenantImpl* self, - Reference tr, - TenantMapEntry tenantEntry) { - // Erase the tenant entry itself - ManagementClusterMetadata::tenantMetadata().tenantMap.erase(tr, self->oldName); - ManagementClusterMetadata::tenantMetadata().lastTenantModification.setVersionstamp(tr, Versionstamp(), 0); - - // Remove old tenant from tenant count - ManagementClusterMetadata::tenantMetadata().tenantCount.atomicOp(tr, -1, MutationRef::AddValue); - ManagementClusterMetadata::clusterTenantCount.atomicOp( - tr, tenantEntry.assignedCluster.get(), -1, MutationRef::AddValue); - - // Clean up cluster based tenant indices and remove the old entry from its tenant group - // Remove the tenant from the cluster -> tenant index - ManagementClusterMetadata::clusterTenantIndex.erase( - tr, Tuple::makeTuple(tenantEntry.assignedCluster.get(), self->oldName, self->tenantId)); - - // Remove the tenant from its tenant group - wait(managementClusterRemoveTenantFromGroup( - tr, self->oldName, tenantEntry, &self->ctx.dataClusterMetadata.get(), true)); - - return Void(); - } - ACTOR static Future markTenantsInRenamingState(RenameTenantImpl* self, Reference tr) { - state TenantMapEntry oldTenantEntry; - state Optional newTenantEntry; - wait(store(oldTenantEntry, getTenantTransaction(tr, self->oldName)) && - store(newTenantEntry, tryGetTenantTransaction(tr, self->newName))); + state TenantMapEntry tenantEntry; + state Optional newNameId; + wait(store(tenantEntry, getTenantTransaction(tr, self->oldName)) && + store(newNameId, ManagementClusterMetadata::tenantMetadata().tenantNameIndex.get(tr, self->newName))); - if (self->tenantId != -1 && oldTenantEntry.id != self->tenantId) { + if (self->tenantId != -1 && tenantEntry.id != self->tenantId) { // The tenant must have been removed simultaneously CODE_PROBE(true, "Metacluster rename old tenant ID mismatch"); throw tenant_removed(); } + self->tenantId = tenantEntry.id; + // If marked for deletion, abort the rename - if (oldTenantEntry.tenantState == TenantState::REMOVING) { + if (tenantEntry.tenantState == TenantState::REMOVING) { CODE_PROBE(true, "Metacluster rename candidates marked for deletion"); throw tenant_removed(); } - // If the new entry is present, we can only continue if this is a retry of the same rename - // To check this, verify both entries are in the correct state - // and have each other as pairs - if (newTenantEntry.present()) { - if (newTenantEntry.get().tenantState == TenantState::RENAMING_TO && - oldTenantEntry.tenantState == TenantState::RENAMING_FROM && newTenantEntry.get().renamePair.present() && - newTenantEntry.get().renamePair.get() == self->oldName && oldTenantEntry.renamePair.present() && - oldTenantEntry.renamePair.get() == self->newName) { - wait(self->ctx.setCluster(tr, oldTenantEntry.assignedCluster.get())); - self->tenantId = newTenantEntry.get().id; - self->configurationSequenceNum = newTenantEntry.get().configurationSequenceNum; - CODE_PROBE(true, "Metacluster rename retry in progress"); - return Void(); - } else { - CODE_PROBE(true, "Metacluster rename new name already exists"); - throw tenant_already_exists(); - }; - } else { - if (self->tenantId == -1) { - self->tenantId = oldTenantEntry.id; + if (newNameId.present() && (newNameId.get() != self->tenantId || self->oldName == self->newName)) { + CODE_PROBE(true, "Metacluster rename new name already exists"); + throw tenant_already_exists(); + } + + wait(self->ctx.setCluster(tr, tenantEntry.assignedCluster.get())); + + if (tenantEntry.tenantState == TenantState::RENAMING) { + if (tenantEntry.tenantName != self->oldName) { + CODE_PROBE(true, "Renaming a tenant that is currently the destination of another rename"); + throw tenant_not_found(); } - ++oldTenantEntry.configurationSequenceNum; - self->configurationSequenceNum = oldTenantEntry.configurationSequenceNum; - wait(self->ctx.setCluster(tr, oldTenantEntry.assignedCluster.get())); - if (oldTenantEntry.tenantState != TenantState::READY) { - CODE_PROBE(true, "Metacluster unable to proceed with rename operation"); - throw invalid_tenant_state(); + if (tenantEntry.renameDestination.get() != self->newName) { + CODE_PROBE(true, "Metacluster concurrent rename with different name"); + throw tenant_already_exists(); + } else { + CODE_PROBE(true, "Metacluster rename retry in progress"); + self->configurationSequenceNum = tenantEntry.configurationSequenceNum; + return Void(); } } + if (tenantEntry.tenantState != TenantState::READY) { + CODE_PROBE(true, "Metacluster unable to proceed with rename operation"); + throw invalid_tenant_state(); + } + + self->configurationSequenceNum = tenantEntry.configurationSequenceNum + 1; // Check cluster capacity. If we would exceed the amount due to temporary extra tenants // then we deny the rename request altogether. int64_t clusterTenantCount = wait(ManagementClusterMetadata::clusterTenantCount.getD( - tr, oldTenantEntry.assignedCluster.get(), Snapshot::False, 0)); + tr, tenantEntry.assignedCluster.get(), Snapshot::False, 0)); if (clusterTenantCount + 1 > CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER) { throw cluster_no_capacity(); } - TenantMapEntry updatedOldEntry = oldTenantEntry; - TenantMapEntry updatedNewEntry(updatedOldEntry); - ASSERT(updatedOldEntry.configurationSequenceNum == self->configurationSequenceNum); - ASSERT(updatedNewEntry.configurationSequenceNum == self->configurationSequenceNum); - updatedOldEntry.tenantState = TenantState::RENAMING_FROM; - updatedNewEntry.tenantState = TenantState::RENAMING_TO; - updatedOldEntry.renamePair = self->newName; - updatedNewEntry.renamePair = self->oldName; + TenantMapEntry updatedEntry = tenantEntry; + updatedEntry.tenantState = TenantState::RENAMING; + updatedEntry.renameDestination = self->newName; + updatedEntry.configurationSequenceNum = self->configurationSequenceNum; - ManagementClusterMetadata::tenantMetadata().tenantMap.set(tr, self->oldName, updatedOldEntry); - ManagementClusterMetadata::tenantMetadata().tenantMap.set(tr, self->newName, updatedNewEntry); + ManagementClusterMetadata::tenantMetadata().tenantMap.set(tr, self->tenantId, updatedEntry); + ManagementClusterMetadata::tenantMetadata().tenantNameIndex.set(tr, self->newName, self->tenantId); ManagementClusterMetadata::tenantMetadata().lastTenantModification.setVersionstamp(tr, Versionstamp(), 0); - // Add temporary tenant to tenantCount to prevent exceeding capacity during a rename - ManagementClusterMetadata::tenantMetadata().tenantCount.atomicOp(tr, 1, MutationRef::AddValue); - ManagementClusterMetadata::clusterTenantCount.atomicOp( - tr, updatedNewEntry.assignedCluster.get(), 1, MutationRef::AddValue); - // Updated indexes to include the new tenant ManagementClusterMetadata::clusterTenantIndex.insert( - tr, Tuple::makeTuple(updatedNewEntry.assignedCluster.get(), self->newName, self->tenantId)); + tr, Tuple::makeTuple(updatedEntry.assignedCluster.get(), self->newName, self->tenantId)); - // Add new name to tenant group. It should already exist since the old name was part of it. - managementClusterAddTenantToGroup( - tr, self->newName, updatedNewEntry, &self->ctx.dataClusterMetadata.get(), true); return Void(); } @@ -1953,44 +1898,40 @@ struct RenameTenantImpl { ACTOR static Future finishRenameFromManagementCluster(RenameTenantImpl* self, Reference tr) { - state Optional oldTenantEntry; - state Optional newTenantEntry; - wait(store(oldTenantEntry, tryGetTenantTransaction(tr, self->oldName)) && - store(newTenantEntry, tryGetTenantTransaction(tr, self->newName))); + Optional tenantEntry = wait(tryGetTenantTransaction(tr, self->tenantId)); // Another (or several other) operations have already removed/changed the old entry // Possible for the new entry to also have been tampered with, // so it may or may not be present with or without the same id, which are all // legal states. Assume the rename completed properly in this case - if (!oldTenantEntry.present() || oldTenantEntry.get().id != self->tenantId || - oldTenantEntry.get().configurationSequenceNum > self->configurationSequenceNum) { + if (!tenantEntry.present() || tenantEntry.get().tenantName != self->oldName || + tenantEntry.get().configurationSequenceNum > self->configurationSequenceNum) { CODE_PROBE(true, "Metacluster finished rename with missing entries, mismatched id, and/or mismatched " "configuration sequence."); return Void(); } - if (oldTenantEntry.get().tenantState == TenantState::REMOVING) { - ASSERT(newTenantEntry.get().tenantState == TenantState::REMOVING); + if (tenantEntry.get().tenantState == TenantState::REMOVING) { throw tenant_removed(); } - ASSERT(newTenantEntry.present()); - ASSERT(newTenantEntry.get().id == self->tenantId); - TenantMapEntry updatedOldEntry = oldTenantEntry.get(); - TenantMapEntry updatedNewEntry = newTenantEntry.get(); + TenantMapEntry updatedEntry = tenantEntry.get(); // Only update if in the expected state - if (updatedNewEntry.tenantState == TenantState::RENAMING_TO) { - updatedNewEntry.tenantState = TenantState::READY; - updatedNewEntry.renamePair.reset(); - ManagementClusterMetadata::tenantMetadata().tenantMap.set(tr, self->newName, updatedNewEntry); - ManagementClusterMetadata::tenantMetadata().tenantIdIndex.set(tr, self->tenantId, self->newName); + if (updatedEntry.tenantState == TenantState::RENAMING) { + updatedEntry.tenantName = self->newName; + updatedEntry.tenantState = TenantState::READY; + updatedEntry.renameDestination.reset(); + ManagementClusterMetadata::tenantMetadata().tenantMap.set(tr, self->tenantId, updatedEntry); ManagementClusterMetadata::tenantMetadata().lastTenantModification.setVersionstamp(tr, Versionstamp(), 0); + + ManagementClusterMetadata::tenantMetadata().tenantNameIndex.erase(tr, self->oldName); + + // Remove the tenant from the cluster -> tenant index + ManagementClusterMetadata::clusterTenantIndex.erase( + tr, Tuple::makeTuple(updatedEntry.assignedCluster.get(), self->oldName, self->tenantId)); } - // We will remove the old entry from the management cluster - // This should still be the same old entry since the tenantId matches from the check above. - wait(deleteTenantFromManagementCluster(self, tr, updatedOldEntry)); return Void(); } diff --git a/fdbclient/include/fdbclient/Tenant.h b/fdbclient/include/fdbclient/Tenant.h index 04019d339b..cbd20972d4 100644 --- a/fdbclient/include/fdbclient/Tenant.h +++ b/fdbclient/include/fdbclient/Tenant.h @@ -49,19 +49,17 @@ constexpr static int PREFIX_SIZE = sizeof(int64_t); // REMOVING - the tenant has been marked for removal and is being removed on the data cluster // UPDATING_CONFIGURATION - the tenant configuration has changed on the management cluster and is being applied to the // data cluster -// RENAMING_FROM - the tenant is being renamed to a new name and is awaiting the rename to complete on the data cluster -// RENAMING_TO - the tenant is being created as a rename from an existing tenant and is awaiting the rename to complete -// on the data cluster +// RENAMING - the tenant is in the process of being renamed // ERROR - the tenant is in an error state // // A tenant in any configuration is allowed to be removed. Only tenants in the READY or UPDATING_CONFIGURATION phases // can have their configuration updated. A tenant must not exist or be in the REGISTERING phase to be created. To be -// renamed, a tenant must be in the READY or RENAMING_FROM state. In the latter case, the rename destination must match +// renamed, a tenant must be in the READY or RENAMING state. In the latter case, the rename destination must match // the original rename attempt. // // If an operation fails and the tenant is left in a non-ready state, re-running the same operation is legal. If // successful, the tenant will return to the READY state. -enum class TenantState { REGISTERING, READY, REMOVING, UPDATING_CONFIGURATION, RENAMING_FROM, RENAMING_TO, ERROR }; +enum class TenantState { REGISTERING, READY, REMOVING, UPDATING_CONFIGURATION, RENAMING, ERROR }; // Represents the lock state the tenant could be in. // Can be used in conjunction with the other tenant states above. @@ -84,7 +82,7 @@ struct TenantMapEntry { Optional tenantGroup; Optional assignedCluster; int64_t configurationSequenceNum = 0; - Optional renamePair; + Optional renameDestination; // Can be set to an error string if the tenant is in the ERROR state std::string error; @@ -114,7 +112,7 @@ struct TenantMapEntry { tenantGroup, assignedCluster, configurationSequenceNum, - renamePair, + renameDestination, error); if constexpr (Ar::isDeserializing) { if (id >= 0) { @@ -165,11 +163,37 @@ struct TenantTombstoneCleanupData { } }; +// This is used so that tenant IDs will be ordered and so that we can easily map arbitrary ranges in the tenant map to +// the affected tenant IDs. +struct TenantIdCodec { + static Standalone pack(int64_t val) { + int64_t swapped = bigEndian64(val); + return StringRef((uint8_t*)&swapped, sizeof(swapped)); + } + static int64_t unpack(Standalone val) { return bigEndian64(*(int64_t*)val.begin()); } + + static Optional lowerBound(Standalone val) { + if (val.size() == 8) { + return unpack(val); + } else if (val.size() > 8) { + int64_t result = unpack(val); + if (result == std::numeric_limits::max()) { + return {}; + } + return result + 1; + } else { + int64_t result = 0; + memcpy(&result, val.begin(), val.size()); + return bigEndian64(result); + } + } +}; + struct TenantMetadataSpecification { Key subspace; - KeyBackedObjectMap tenantMap; - KeyBackedMap tenantIdIndex; + KeyBackedObjectMap tenantMap; + KeyBackedMap tenantNameIndex; KeyBackedProperty lastTenantId; KeyBackedBinaryValue tenantCount; KeyBackedSet tenantTombstones; @@ -180,7 +204,7 @@ struct TenantMetadataSpecification { TenantMetadataSpecification(KeyRef prefix) : subspace(prefix.withSuffix("tenant/"_sr)), tenantMap(subspace.withSuffix("map/"_sr), IncludeVersion()), - tenantIdIndex(subspace.withSuffix("idIndex/"_sr)), lastTenantId(subspace.withSuffix("lastId"_sr)), + tenantNameIndex(subspace.withSuffix("nameIndex/"_sr)), lastTenantId(subspace.withSuffix("lastId"_sr)), tenantCount(subspace.withSuffix("count"_sr)), tenantTombstones(subspace.withSuffix("tombstones/"_sr)), tombstoneCleanupData(subspace.withSuffix("tombstoneCleanup"_sr), IncludeVersion()), tenantGroupTenantIndex(subspace.withSuffix("tenantGroup/tenantIndex/"_sr)), @@ -193,7 +217,7 @@ struct TenantMetadata { static inline auto& subspace() { return instance().subspace; } static inline auto& tenantMap() { return instance().tenantMap; } - static inline auto& tenantIdIndex() { return instance().tenantIdIndex; } + static inline auto& tenantNameIndex() { return instance().tenantNameIndex; } static inline auto& lastTenantId() { return instance().lastTenantId; } static inline auto& tenantCount() { return instance().tenantCount; } static inline auto& tenantTombstones() { return instance().tenantTombstones; } diff --git a/fdbclient/include/fdbclient/TenantEntryCache.actor.h b/fdbclient/include/fdbclient/TenantEntryCache.actor.h index a167581167..199bdb04fa 100644 --- a/fdbclient/include/fdbclient/TenantEntryCache.actor.h +++ b/fdbclient/include/fdbclient/TenantEntryCache.actor.h @@ -55,14 +55,13 @@ enum class TenantEntryCacheRefreshMode { PERIODIC_TASK = 1, WATCH = 2, NONE = 3 template struct TenantEntryCachePayload { - TenantName name; TenantMapEntry entry; // Custom client payload T payload; }; template -using TenantEntryCachePayloadFunc = std::function(const TenantName&, const TenantMapEntry&)>; +using TenantEntryCachePayloadFunc = std::function(const TenantMapEntry&)>; // In-memory cache for TenantEntryMap objects. It supports three indices: // 1. Lookup by 'TenantId' @@ -97,13 +96,13 @@ private: Counter numRefreshes; Counter refreshByWatchTrigger; - ACTOR static Future getTenantList(Reference tr) { + ACTOR static Future>> getTenantList( + Reference tr) { tr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS); tr->setOption(FDBTransactionOptions::READ_LOCK_AWARE); - KeyBackedRangeResult> tenantList = - wait(TenantMetadata::tenantMap().getRange( - tr, Optional(), Optional(), CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER + 1)); + KeyBackedRangeResult> tenantList = + wait(TenantMetadata::tenantMap().getRange(tr, {}, {}, CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER + 1)); ASSERT(tenantList.results.size() <= CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER && !tenantList.more); TraceEvent(SevDebug, "TenantEntryCacheGetTenantList").detail("Count", tenantList.results.size()); @@ -120,13 +119,10 @@ private: try { tr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS); tr->setOption(FDBTransactionOptions::READ_LOCK_AWARE); - state Optional name = wait(TenantMetadata::tenantIdIndex().get(tr, tenantId)); - if (name.present()) { - Optional entry = wait(TenantMetadata::tenantMap().get(tr, name.get())); - if (entry.present()) { - cache->put(std::make_pair(name.get(), entry.get())); - updateCacheRefreshMetrics(cache, reason); - } + state Optional entry = wait(TenantMetadata::tenantMap().get(tr, tenantId)); + if (entry.present()) { + cache->put(entry.get()); + updateCacheRefreshMetrics(cache, reason); } break; } catch (Error& e) { @@ -147,10 +143,13 @@ private: try { tr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS); tr->setOption(FDBTransactionOptions::READ_LOCK_AWARE); - Optional entry = wait(TenantMetadata::tenantMap().get(tr, name)); - if (entry.present()) { - cache->put(std::make_pair(name, entry.get())); - updateCacheRefreshMetrics(cache, reason); + state Optional tenantId = wait(TenantMetadata::tenantNameIndex().get(tr, name)); + if (tenantId.present()) { + Optional entry = wait(TenantMetadata::tenantMap().get(tr, tenantId.get())); + if (entry.present()) { + cache->put(entry.get()); + updateCacheRefreshMetrics(cache, reason); + } } break; } catch (Error& e) { @@ -277,12 +276,12 @@ private: state Reference tr = cache->getDatabase()->createTransaction(); loop { try { - state TenantNameEntryPairVec tenantList = wait(getTenantList(tr)); + state std::vector> tenantList = wait(getTenantList(tr)); // Refresh cache entries reflecting the latest database state cache->clear(); for (auto& tenant : tenantList) { - cache->put(tenant); + cache->put(tenant.second); } updateCacheRefreshMetrics(cache, reason); @@ -378,9 +377,8 @@ private: Future refresh(TenantEntryCacheRefreshReason reason) { return refreshImpl(this, reason); } - static TenantEntryCachePayload defaultCreatePayload(const TenantName& name, const TenantMapEntry& entry) { + static TenantEntryCachePayload defaultCreatePayload(const TenantMapEntry& entry) { TenantEntryCachePayload payload; - payload.name = name; payload.entry = entry; return payload; @@ -405,7 +403,7 @@ private: return Void(); } // Ensure byId and byName cache are in-sync - itrName = mapByTenantName.find(itrId->value.name); + itrName = mapByTenantName.find(itrId->value.entry.tenantName); ASSERT(itrName != mapByTenantName.end()); } else if (tenantName.present()) { ASSERT(!tenantId.present() && !tenantPrefix.present()); @@ -538,11 +536,10 @@ public: return removeEntryInt(Optional(), Optional(), tenantName, refreshCache); } - void put(const TenantNameEntryPair& pair) { - const auto& [name, entry] = pair; - TenantEntryCachePayload payload = createPayloadFunc(name, entry); + void put(const TenantMapEntry& entry) { + TenantEntryCachePayload payload = createPayloadFunc(entry); auto idItr = mapByTenantId.find(entry.id); - auto nameItr = mapByTenantName.find(name); + auto nameItr = mapByTenantName.find(entry.tenantName); Optional existingName; Optional existingId; @@ -550,7 +547,7 @@ public: existingId = nameItr->value.entry.id; } if (idItr != mapByTenantId.end()) { - existingName = idItr->value.name; + existingName = idItr->value.entry.tenantName; } if (existingId.present()) { mapByTenantId.erase(existingId.get()); @@ -560,14 +557,14 @@ public: } mapByTenantId[entry.id] = payload; - mapByTenantName[name] = payload; + mapByTenantName[entry.tenantName] = payload; TraceEvent("TenantEntryCachePut") - .detail("TenantName", name) + .detail("TenantName", entry.tenantName) .detail("TenantNameExisting", existingName) .detail("TenantID", entry.id) .detail("TenantIDExisting", existingId) - .detail("TenantPrefix", pair.second.prefix); + .detail("TenantPrefix", entry.prefix); CODE_PROBE(idItr == mapByTenantId.end() && nameItr == mapByTenantName.end(), "TenantCache new entry"); CODE_PROBE(idItr != mapByTenantId.end() && nameItr == mapByTenantName.end(), "TenantCache entry name updated"); @@ -591,4 +588,4 @@ public: }; #include "flow/unactorcompiler.h" -#endif // FDBCLIENT_TENANTENTRYCACHE_ACTOR_H +#endif // FDBCLIENT_TENANTENTRYCACHE_ACTOR_H \ No newline at end of file diff --git a/fdbclient/include/fdbclient/TenantManagement.actor.h b/fdbclient/include/fdbclient/TenantManagement.actor.h index fb2540fc81..44f61c1a75 100644 --- a/fdbclient/include/fdbclient/TenantManagement.actor.h +++ b/fdbclient/include/fdbclient/TenantManagement.actor.h @@ -38,20 +38,32 @@ namespace TenantAPI { template -Future> tryGetTenantTransaction(Transaction tr, TenantName name) { +Future> tryGetTenantTransaction(Transaction tr, int64_t tenantId) { tr->setOption(FDBTransactionOptions::RAW_ACCESS); - return TenantMetadata::tenantMap().get(tr, name); + return TenantMetadata::tenantMap().get(tr, tenantId); } -ACTOR template -Future> tryGetTenant(Reference db, TenantName name) { +ACTOR template +Future> tryGetTenantTransaction(Transaction tr, TenantName name) { + tr->setOption(FDBTransactionOptions::RAW_ACCESS); + Optional tenantId = wait(TenantMetadata::tenantNameIndex().get(tr, name)); + if (tenantId.present()) { + Optional entry = wait(TenantMetadata::tenantMap().get(tr, tenantId.get())); + return entry; + } else { + return Optional(); + } +} + +ACTOR template +Future> tryGetTenant(Reference db, Tenant tenant) { state Reference tr = db->createTransaction(); loop { try { tr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS); tr->setOption(FDBTransactionOptions::READ_LOCK_AWARE); - Optional entry = wait(tryGetTenantTransaction(tr, name)); + Optional entry = wait(tryGetTenantTransaction(tr, tenant)); return entry; } catch (Error& e) { wait(safeThreadFutureToFuture(tr->onError(e))); @@ -59,9 +71,9 @@ Future> tryGetTenant(Reference db, TenantName name) } } -ACTOR template -Future getTenantTransaction(Transaction tr, TenantName name) { - Optional entry = wait(tryGetTenantTransaction(tr, name)); +ACTOR template +Future getTenantTransaction(Transaction tr, Tenant tenant) { + Optional entry = wait(tryGetTenantTransaction(tr, tenant)); if (!entry.present()) { throw tenant_not_found(); } @@ -69,9 +81,9 @@ Future getTenantTransaction(Transaction tr, TenantName name) { return entry.get(); } -ACTOR template -Future getTenant(Reference db, TenantName name) { - Optional entry = wait(tryGetTenant(db, name)); +ACTOR template +Future getTenant(Reference db, Tenant tenant) { + Optional entry = wait(tryGetTenant(db, tenant)); if (!entry.present()) { throw tenant_not_found(); } @@ -126,29 +138,24 @@ Future checkTombstone(Transaction tr, int64_t id) { return hasTombstone; } -// Creates a tenant with the given name. If the tenant already exists, the boolean return parameter will be false +// Creates a tenant. If the tenant already exists, the boolean return parameter will be false // and the existing entry will be returned. If the tenant cannot be created, then the optional will be empty. ACTOR template -Future, bool>> createTenantTransaction( - Transaction tr, - TenantNameRef name, - TenantMapEntry tenantEntry, - ClusterType clusterType = ClusterType::STANDALONE) { - +Future, bool>> +createTenantTransaction(Transaction tr, TenantMapEntry tenantEntry, ClusterType clusterType = ClusterType::STANDALONE) { ASSERT(clusterType != ClusterType::METACLUSTER_MANAGEMENT); ASSERT(tenantEntry.id >= 0); - if (name.startsWith("\xff"_sr)) { + if (tenantEntry.tenantName.startsWith("\xff"_sr)) { throw invalid_tenant_name(); } if (tenantEntry.tenantGroup.present() && tenantEntry.tenantGroup.get().startsWith("\xff"_sr)) { throw invalid_tenant_group_name(); } - tenantEntry.tenantName = name; tr->setOption(FDBTransactionOptions::RAW_ACCESS); - state Future> existingEntryFuture = tryGetTenantTransaction(tr, name); + state Future> existingEntryFuture = tryGetTenantTransaction(tr, tenantEntry.tenantName); state Future tenantModeCheck = checkTenantMode(tr, clusterType); state Future tombstoneFuture = (clusterType == ClusterType::STANDALONE) ? false : checkTombstone(tr, tenantEntry.id); @@ -179,12 +186,13 @@ Future, bool>> createTenantTransaction( tenantEntry.tenantState = TenantState::READY; tenantEntry.assignedCluster = Optional(); - TenantMetadata::tenantMap().set(tr, name, tenantEntry); - TenantMetadata::tenantIdIndex().set(tr, tenantEntry.id, name); + TenantMetadata::tenantMap().set(tr, tenantEntry.id, tenantEntry); + TenantMetadata::tenantNameIndex().set(tr, tenantEntry.tenantName, tenantEntry.id); TenantMetadata::lastTenantModification().setVersionstamp(tr, Versionstamp(), 0); if (tenantEntry.tenantGroup.present()) { - TenantMetadata::tenantGroupTenantIndex().insert(tr, Tuple::makeTuple(tenantEntry.tenantGroup.get(), name)); + TenantMetadata::tenantGroupTenantIndex().insert( + tr, Tuple::makeTuple(tenantEntry.tenantGroup.get(), tenantEntry.id)); // Create the tenant group associated with this tenant if it doesn't already exist Optional existingTenantGroup = wait(existingTenantGroupEntryFuture); @@ -228,6 +236,8 @@ Future> createTenant(Reference db, ASSERT(clusterType == ClusterType::STANDALONE || !generateTenantId); + tenantEntry.tenantName = name; + loop { try { tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); @@ -239,8 +249,8 @@ Future> createTenant(Reference db, } if (checkExistence) { - Optional entry = wait(tryGetTenantTransaction(tr, name)); - if (entry.present()) { + Optional existingId = wait(TenantMetadata::tenantNameIndex().get(tr, name)); + if (existingId.present()) { throw tenant_already_exists(); } @@ -254,7 +264,7 @@ Future> createTenant(Reference db, } state std::pair, bool> newTenant = - wait(createTenantTransaction(tr, name, tenantEntry, clusterType)); + wait(createTenantTransaction(tr, tenantEntry, clusterType)); if (newTenant.second) { ASSERT(newTenant.first.present()); @@ -319,25 +329,23 @@ Future markTenantTombstones(Transaction tr, int64_t tenantId) { return Void(); } -// Deletes the tenant with the given name. If tenantId is specified, the tenant being deleted must also have the same -// ID. If no matching tenant is found, this function returns without deleting anything. This behavior allows the -// function to be used idempotently: if the transaction is retried after having succeeded, it will see that the tenant -// is absent (or optionally created with a new ID) and do nothing. +// Deletes a tenant with the given ID. If no matching tenant is found, this function returns without deleting anything. +// This behavior allows the function to be used idempotently: if the transaction is retried after having succeeded, it +// will see that the tenant is absent and do nothing. ACTOR template Future deleteTenantTransaction(Transaction tr, - TenantNameRef name, - Optional tenantId = Optional(), + int64_t tenantId, ClusterType clusterType = ClusterType::STANDALONE) { - ASSERT(clusterType == ClusterType::STANDALONE || tenantId.present()); + ASSERT(tenantId != TenantInfo::INVALID_TENANT); ASSERT(clusterType != ClusterType::METACLUSTER_MANAGEMENT); tr->setOption(FDBTransactionOptions::RAW_ACCESS); - state Future> tenantEntryFuture = tryGetTenantTransaction(tr, name); + state Future> tenantEntryFuture = tryGetTenantTransaction(tr, tenantId); wait(checkTenantMode(tr, clusterType)); state Optional tenantEntry = wait(tenantEntryFuture); - if (tenantEntry.present() && (!tenantId.present() || tenantEntry.get().id == tenantId.get())) { + if (tenantEntry.present()) { state typename transaction_future_type::type prefixRangeFuture = tr->getRange(prefixRange(tenantEntry.get().prefix), 1); @@ -347,14 +355,14 @@ Future deleteTenantTransaction(Transaction tr, } // This is idempotent because we only erase an entry from the tenant map if it is present - TenantMetadata::tenantMap().erase(tr, name); - TenantMetadata::tenantIdIndex().erase(tr, tenantEntry.get().id); + TenantMetadata::tenantMap().erase(tr, tenantId); + TenantMetadata::tenantNameIndex().erase(tr, tenantEntry.get().tenantName); TenantMetadata::tenantCount().atomicOp(tr, -1, MutationRef::AddValue); TenantMetadata::lastTenantModification().setVersionstamp(tr, Versionstamp(), 0); if (tenantEntry.get().tenantGroup.present()) { - TenantMetadata::tenantGroupTenantIndex().erase(tr, - Tuple::makeTuple(tenantEntry.get().tenantGroup.get(), name)); + TenantMetadata::tenantGroupTenantIndex().erase( + tr, Tuple::makeTuple(tenantEntry.get().tenantGroup.get(), tenantId)); KeyBackedSet::RangeResultType tenantsInGroup = wait(TenantMetadata::tenantGroupTenantIndex().getRange( tr, @@ -362,14 +370,14 @@ Future deleteTenantTransaction(Transaction tr, Tuple::makeTuple(keyAfter(tenantEntry.get().tenantGroup.get())), 2)); if (tenantsInGroup.results.empty() || - (tenantsInGroup.results.size() == 1 && tenantsInGroup.results[0].getString(1) == name)) { + (tenantsInGroup.results.size() == 1 && tenantsInGroup.results[0].getInt(1) == tenantId)) { TenantMetadata::tenantGroupMap().erase(tr, tenantEntry.get().tenantGroup.get()); } } } if (clusterType == ClusterType::METACLUSTER_DATA) { - wait(markTenantTombstones(tr, tenantId.get())); + wait(markTenantTombstones(tr, tenantId)); } return Void(); @@ -391,21 +399,22 @@ Future deleteTenant(Reference db, tr->setOption(FDBTransactionOptions::LOCK_AWARE); if (checkExistence) { - TenantMapEntry entry = wait(getTenantTransaction(tr, name)); - - // If an ID wasn't specified, use the current ID. This way we cannot inadvertently delete - // multiple tenants if this transaction retries. - if (!tenantId.present()) { - tenantId = entry.id; + Optional actualId = wait(TenantMetadata::tenantNameIndex().get(tr, name)); + if (!actualId.present() || (tenantId.present() && tenantId != actualId)) { + throw tenant_not_found(); } + tenantId = actualId; checkExistence = false; } - wait(deleteTenantTransaction(tr, name, tenantId, clusterType)); + wait(deleteTenantTransaction(tr, tenantId.get(), clusterType)); wait(buggifiedCommit(tr, BUGGIFY_WITH_PROB(0.1))); - TraceEvent("DeletedTenant").detail("Tenant", name).detail("Version", tr->getCommittedVersion()); + TraceEvent("DeletedTenant") + .detail("Tenant", name) + .detail("TenantId", tenantId) + .detail("Version", tr->getCommittedVersion()); return Void(); } catch (Error& e) { wait(safeThreadFutureToFuture(tr->onError(e))); @@ -418,13 +427,12 @@ Future deleteTenant(Reference db, // to be changed. This must only be called on a non-management cluster. ACTOR template Future configureTenantTransaction(Transaction tr, - TenantNameRef tenantName, TenantMapEntry originalEntry, TenantMapEntry updatedTenantEntry) { ASSERT(updatedTenantEntry.id == originalEntry.id); tr->setOption(FDBTransactionOptions::RAW_ACCESS); - TenantMetadata::tenantMap().set(tr, tenantName, updatedTenantEntry); + TenantMetadata::tenantMap().set(tr, updatedTenantEntry.id, updatedTenantEntry); TenantMetadata::lastTenantModification().setVersionstamp(tr, Versionstamp(), 0); // If the tenant group was changed, we need to update the tenant group metadata structures @@ -435,7 +443,7 @@ Future configureTenantTransaction(Transaction tr, if (originalEntry.tenantGroup.present()) { // Remove this tenant from the original tenant group index TenantMetadata::tenantGroupTenantIndex().erase( - tr, Tuple::makeTuple(originalEntry.tenantGroup.get(), tenantName)); + tr, Tuple::makeTuple(originalEntry.tenantGroup.get(), updatedTenantEntry.id)); // Check if the original tenant group is now empty. If so, remove the tenant group. KeyBackedSet::RangeResultType tenants = wait(TenantMetadata::tenantGroupTenantIndex().getRange( @@ -445,7 +453,7 @@ Future configureTenantTransaction(Transaction tr, 2)); if (tenants.results.empty() || - (tenants.results.size() == 1 && tenants.results[0].getString(1) == tenantName)) { + (tenants.results.size() == 1 && tenants.results[0].getInt(1) == updatedTenantEntry.id)) { TenantMetadata::tenantGroupMap().erase(tr, originalEntry.tenantGroup.get()); } } @@ -459,7 +467,7 @@ Future configureTenantTransaction(Transaction tr, // Insert this tenant in the tenant group index TenantMetadata::tenantGroupTenantIndex().insert( - tr, Tuple::makeTuple(updatedTenantEntry.tenantGroup.get(), tenantName)); + tr, Tuple::makeTuple(updatedTenantEntry.tenantGroup.get(), updatedTenantEntry.id)); } } @@ -473,10 +481,22 @@ Future>> listTenantsTransactio int limit) { tr->setOption(FDBTransactionOptions::RAW_ACCESS); - KeyBackedRangeResult> results = - wait(TenantMetadata::tenantMap().getRange(tr, begin, end, limit)); + KeyBackedRangeResult> matchingTenants = + wait(TenantMetadata::tenantNameIndex().getRange(tr, begin, end, limit)); - return results.results; + state std::vector> tenantEntryFutures; + for (auto const& [name, id] : matchingTenants.results) { + tenantEntryFutures.push_back(getTenantTransaction(tr, id)); + } + + wait(waitForAll(tenantEntryFutures)); + + std::vector> results; + for (auto const& f : tenantEntryFutures) { + results.emplace_back(f.get().tenantName, f.get()); + } + + return results; } ACTOR template @@ -508,35 +528,45 @@ Future renameTenantTransaction(Transaction tr, Optional configureSequenceNum = Optional()) { ASSERT(clusterType == ClusterType::STANDALONE || (tenantId.present() && configureSequenceNum.present())); ASSERT(clusterType != ClusterType::METACLUSTER_MANAGEMENT); - wait(checkTenantMode(tr, clusterType)); + tr->setOption(FDBTransactionOptions::RAW_ACCESS); - state Optional oldEntry; - state Optional newEntry; - wait(store(oldEntry, tryGetTenantTransaction(tr, oldName)) && - store(newEntry, tryGetTenantTransaction(tr, newName))); - if (!oldEntry.present() || (tenantId.present() && tenantId.get() != oldEntry.get().id)) { + + state Future tenantModeCheck = checkTenantMode(tr, clusterType); + state Future> oldNameIdFuture = + tenantId.present() ? Future>() : TenantMetadata::tenantNameIndex().get(tr, oldName); + state Future> newNameIdFuture = TenantMetadata::tenantNameIndex().get(tr, newName); + + wait(tenantModeCheck); + + if (!tenantId.present()) { + wait(store(tenantId, oldNameIdFuture)); + if (!tenantId.present()) { + throw tenant_not_found(); + } + } + + state TenantMapEntry entry = wait(getTenantTransaction(tr, tenantId.get())); + Optional newNameId = wait(newNameIdFuture); + if (entry.tenantName != oldName) { throw tenant_not_found(); } - if (newEntry.present()) { + if (newNameId.present()) { throw tenant_already_exists(); } + if (configureSequenceNum.present()) { - if (oldEntry.get().configurationSequenceNum >= configureSequenceNum.get()) { + if (entry.configurationSequenceNum >= configureSequenceNum.get()) { return Void(); } - oldEntry.get().configurationSequenceNum = configureSequenceNum.get(); + entry.configurationSequenceNum = configureSequenceNum.get(); } - TenantMetadata::tenantMap().erase(tr, oldName); - TenantMetadata::tenantMap().set(tr, newName, oldEntry.get()); - TenantMetadata::tenantIdIndex().set(tr, oldEntry.get().id, newName); - TenantMetadata::lastTenantModification().setVersionstamp(tr, Versionstamp(), 0); - // Update the tenant group index to reflect the new tenant name - if (oldEntry.get().tenantGroup.present()) { - TenantMetadata::tenantGroupTenantIndex().erase(tr, Tuple::makeTuple(oldEntry.get().tenantGroup.get(), oldName)); - TenantMetadata::tenantGroupTenantIndex().insert(tr, - Tuple::makeTuple(oldEntry.get().tenantGroup.get(), newName)); - } + entry.tenantName = newName; + + TenantMetadata::tenantMap().set(tr, tenantId.get(), entry); + TenantMetadata::tenantNameIndex().set(tr, newName, tenantId.get()); + TenantMetadata::tenantNameIndex().erase(tr, oldName); + TenantMetadata::lastTenantModification().setVersionstamp(tr, Versionstamp(), 0); if (clusterType == ClusterType::METACLUSTER_DATA) { wait(markTenantTombstones(tr, tenantId.get())); @@ -555,50 +585,38 @@ Future renameTenant(Reference db, ASSERT(clusterType == ClusterType::STANDALONE || tenantId.present()); state bool firstTry = true; - state int64_t id; loop { try { tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); - state Optional oldEntry; - state Optional newEntry; - wait(store(oldEntry, tryGetTenantTransaction(tr, oldName)) && - store(newEntry, tryGetTenantTransaction(tr, newName))); - if (firstTry) { - if (!oldEntry.present()) { - throw tenant_not_found(); - } - if (newEntry.present()) { - throw tenant_already_exists(); - } - // Store the id we see when first reading this key - id = oldEntry.get().id; - - firstTry = false; - } else { - // If we got commit_unknown_result, the rename may have already occurred. - if (newEntry.present()) { - int64_t checkId = newEntry.get().id; - if (id == checkId) { - ASSERT(!oldEntry.present() || oldEntry.get().id != id); - return Void(); - } - // If the new entry is present but does not match, then - // the rename should fail, so we throw an error. - throw tenant_already_exists(); - } - if (!oldEntry.present()) { - throw tenant_not_found(); - } - int64_t checkId = oldEntry.get().id; - // If the id has changed since we made our first attempt, - // then it's possible we've already moved the tenant. Don't move it again. - if (id != checkId) { + if (!tenantId.present()) { + wait(store(tenantId, TenantMetadata::tenantNameIndex().get(tr, oldName))); + if (!tenantId.present()) { throw tenant_not_found(); } } + + state Future> newNameIdFuture = TenantMetadata::tenantNameIndex().get(tr, newName); + state TenantMapEntry entry = wait(getTenantTransaction(tr, tenantId.get())); + Optional newNameId = wait(newNameIdFuture); + + if (!firstTry && entry.tenantName == newName) { + // On a retry, the rename may have already occurred + return Void(); + } else if (entry.tenantName != oldName) { + throw tenant_not_found(); + } else if (newNameId.present() && newNameId.get() != tenantId.get()) { + throw tenant_already_exists(); + } + + firstTry = false; + wait(renameTenantTransaction(tr, oldName, newName, tenantId, clusterType)); wait(buggifiedCommit(tr, BUGGIFY_WITH_PROB(0.1))); - TraceEvent("RenameTenantSuccess").detail("OldName", oldName).detail("NewName", newName); + + TraceEvent("TenantRenamed") + .detail("OldName", oldName) + .detail("NewName", newName) + .detail("TenantId", tenantId.get()); return Void(); } catch (Error& e) { wait(safeThreadFutureToFuture(tr->onError(e))); @@ -664,4 +682,4 @@ Future>> listTenantGrou } // namespace TenantAPI #include "flow/unactorcompiler.h" -#endif +#endif \ No newline at end of file diff --git a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h index 69814d521e..1b8e2bf6c9 100644 --- a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h +++ b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h @@ -110,8 +110,7 @@ private: std::vector, Optional>> configMutations, int64_t tenantId, std::map* tenantGroupNetTenantDelta) { - state TenantMapEntry tenantEntry; - tenantEntry.setId(tenantId); + state TenantMapEntry tenantEntry(tenantId, tenantName, TenantState::READY); for (auto const& [name, value] : configMutations) { tenantEntry.configure(name, value); @@ -122,7 +121,7 @@ private: } std::pair, bool> entry = - wait(TenantAPI::createTenantTransaction(&ryw->getTransaction(), tenantName, tenantEntry)); + wait(TenantAPI::createTenantTransaction(&ryw->getTransaction(), tenantEntry)); return entry.second; } @@ -180,7 +179,7 @@ private: } } - wait(TenantAPI::configureTenantTransaction(&ryw->getTransaction(), tenantName, originalEntry, updatedEntry)); + wait(TenantAPI::configureTenantTransaction(&ryw->getTransaction(), originalEntry, updatedEntry)); return Void(); } @@ -190,7 +189,7 @@ private: state Optional tenantEntry = wait(TenantAPI::tryGetTenantTransaction(&ryw->getTransaction(), tenantName)); if (tenantEntry.present()) { - wait(TenantAPI::deleteTenantTransaction(&ryw->getTransaction(), tenantName)); + wait(TenantAPI::deleteTenantTransaction(&ryw->getTransaction(), tenantEntry.get().id)); if (tenantEntry.get().tenantGroup.present()) { (*tenantGroupNetTenantDelta)[tenantEntry.get().tenantGroup.get()]--; } @@ -217,10 +216,7 @@ private: std::vector> deleteFutures; for (auto tenant : tenants) { - deleteFutures.push_back(TenantAPI::deleteTenantTransaction(&ryw->getTransaction(), tenant.first)); - if (tenant.second.tenantGroup.present()) { - (*tenantGroupNetTenantDelta)[tenant.second.tenantGroup.get()]--; - } + deleteFutures.push_back(deleteSingleTenant(ryw, tenant.first, tenantGroupNetTenantDelta)); } wait(waitForAll(deleteFutures)); diff --git a/fdbserver/ApplyMetadataMutation.cpp b/fdbserver/ApplyMetadataMutation.cpp index d4797895e2..e91e84105e 100644 --- a/fdbserver/ApplyMetadataMutation.cpp +++ b/fdbserver/ApplyMetadataMutation.cpp @@ -137,8 +137,8 @@ private: std::map* tag_popped = nullptr; std::unordered_map* tssMapping = nullptr; - std::unordered_map* tenantMap = nullptr; - std::map* tenantNameIndex = nullptr; + std::map* tenantMap = nullptr; + std::unordered_map* tenantNameIndex = nullptr; EncryptionAtRestMode encryptMode; // true if the mutations were already written to the txnStateStore as part of recovery @@ -672,20 +672,19 @@ private: void checkSetTenantMapPrefix(MutationRef m) { KeyRef prefix = TenantMetadata::tenantMap().subspace.begin; if (m.param1.startsWith(prefix)) { - TenantName tenantName = m.param1.removePrefix(prefix); TenantMapEntry tenantEntry = TenantMapEntry::decode(m.param2); if (tenantMap) { ASSERT(version != invalidVersion); TraceEvent("CommitProxyInsertTenant", dbgid) - .detail("Tenant", tenantName) + .detail("Tenant", tenantEntry.tenantName) .detail("Id", tenantEntry.id) .detail("Version", version); - (*tenantMap)[tenantEntry.id] = tenantName; + (*tenantMap)[tenantEntry.id] = tenantEntry.tenantName; if (tenantNameIndex) { - (*tenantNameIndex)[tenantName] = tenantEntry.id; + (*tenantNameIndex)[tenantEntry.tenantName] = tenantEntry.id; } } @@ -1095,25 +1094,31 @@ private: if (tenantMap && tenantNameIndex) { ASSERT(version != invalidVersion); - StringRef startTenant = std::max(range.begin, subspace.begin).removePrefix(subspace.begin); - StringRef endTenant = - range.end.startsWith(subspace.begin) ? range.end.removePrefix(subspace.begin) : "\xff\xff"_sr; + Optional startId = 0; + Optional endId; + + if (range.begin > subspace.begin) { + startId = TenantIdCodec::lowerBound(range.begin.removePrefix(subspace.begin)); + } + if (range.end.startsWith(subspace.begin)) { + endId = TenantIdCodec::lowerBound(range.end.removePrefix(subspace.begin)); + } TraceEvent("CommitProxyEraseTenants", dbgid) - .detail("BeginTenant", startTenant) - .detail("EndTenant", endTenant) + .detail("BeginTenant", startId) + .detail("EndTenant", endId) .detail("Version", version); - auto startItr = tenantNameIndex->lower_bound(startTenant); - auto endItr = tenantNameIndex->lower_bound(endTenant); + auto startItr = startId.present() ? tenantMap->lower_bound(startId.get()) : tenantMap->end(); + auto endItr = endId.present() ? tenantMap->lower_bound(endId.get()) : tenantMap->end(); auto itr = startItr; while (itr != endItr) { - tenantMap->erase(itr->second); + tenantNameIndex->erase(itr->second); itr++; } - tenantNameIndex->erase(startItr, endItr); + tenantMap->erase(startItr, endItr); } if (!initialCommit) { diff --git a/fdbserver/BlobGranuleServerCommon.actor.cpp b/fdbserver/BlobGranuleServerCommon.actor.cpp index cca9dee623..53fbb748fd 100644 --- a/fdbserver/BlobGranuleServerCommon.actor.cpp +++ b/fdbserver/BlobGranuleServerCommon.actor.cpp @@ -499,11 +499,11 @@ Future loadBlobMetadataForTenant(BGTenantMap* self, BlobMetadataDomainId d } // list of tenants that may or may not already exist -void BGTenantMap::addTenants(std::vector> tenants) { +void BGTenantMap::addTenants(std::vector> tenants) { std::vector tenantsToLoad; for (auto entry : tenants) { - if (tenantInfoById.insert({ entry.second.id, entry.second }).second) { - auto r = makeReference(entry.first, entry.second); + if (tenantInfoById.insert({ entry.first, entry.second }).second) { + auto r = makeReference(entry.second); tenantData.insert(KeyRangeRef(entry.second.prefix, entry.second.prefix.withSuffix(normalKeys.end)), r); if (SERVER_KNOBS->BG_METADATA_SOURCE != "tenant") { r->bstoreLoaded.send(Void()); diff --git a/fdbserver/BlobManager.actor.cpp b/fdbserver/BlobManager.actor.cpp index a00d9e7934..20b8d90165 100644 --- a/fdbserver/BlobManager.actor.cpp +++ b/fdbserver/BlobManager.actor.cpp @@ -1249,10 +1249,9 @@ ACTOR Future writeInitialGranuleMapping(Reference bmData, } ACTOR Future loadTenantMap(Reference tr, Reference bmData) { - state KeyBackedRangeResult> tenantResults; + state KeyBackedRangeResult> tenantResults; wait(store(tenantResults, - TenantMetadata::tenantMap().getRange( - tr, Optional(), Optional(), CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER + 1))); + TenantMetadata::tenantMap().getRange(tr, {}, {}, CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER + 1))); ASSERT(tenantResults.results.size() <= CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER && !tenantResults.more); bmData->tenantData.addTenants(tenantResults.results); diff --git a/fdbserver/BlobWorker.actor.cpp b/fdbserver/BlobWorker.actor.cpp index 69e4355893..f4ac560cd9 100644 --- a/fdbserver/BlobWorker.actor.cpp +++ b/fdbserver/BlobWorker.actor.cpp @@ -4831,15 +4831,13 @@ ACTOR Future monitorTenants(Reference bwData) { tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); tr->setOption(FDBTransactionOptions::PRIORITY_SYSTEM_IMMEDIATE); tr->setOption(FDBTransactionOptions::LOCK_AWARE); - state KeyBackedRangeResult> tenantResults; - wait(store(tenantResults, - TenantMetadata::tenantMap().getRange(tr, - Optional(), - Optional(), - CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER + 1))); + state KeyBackedRangeResult> tenantResults; + wait( + store(tenantResults, + TenantMetadata::tenantMap().getRange(tr, {}, {}, CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER + 1))); ASSERT(tenantResults.results.size() <= CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER && !tenantResults.more); - std::vector> tenants; + std::vector> tenants; for (auto& it : tenantResults.results) { // FIXME: handle removing/moving tenants! tenants.push_back(std::pair(it.first, it.second)); diff --git a/fdbserver/TenantCache.actor.cpp b/fdbserver/TenantCache.actor.cpp index af5f98c1ca..b4b5883970 100644 --- a/fdbserver/TenantCache.actor.cpp +++ b/fdbserver/TenantCache.actor.cpp @@ -37,16 +37,11 @@ class TenantCacheImpl { tr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS); tr->setOption(FDBTransactionOptions::READ_LOCK_AWARE); - KeyBackedRangeResult> tenantList = + KeyBackedRangeResult> tenantList = wait(TenantMetadata::tenantMap().getRange(tr, {}, {}, CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER + 1)); ASSERT(tenantList.results.size() <= CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER && !tenantList.more); - std::vector> results; - for (auto [_, entry] : tenantList.results) { - results.push_back(std::make_pair(entry.id, entry)); - } - - return results; + return tenantList.results; } public: diff --git a/fdbserver/include/fdbserver/BlobGranuleServerCommon.actor.h b/fdbserver/include/fdbserver/BlobGranuleServerCommon.actor.h index b6ff89fbcb..8f7f021b08 100644 --- a/fdbserver/include/fdbserver/BlobGranuleServerCommon.actor.h +++ b/fdbserver/include/fdbserver/BlobGranuleServerCommon.actor.h @@ -97,13 +97,12 @@ ACTOR Future getForcePurgedState(Transaction* tr, KeyRange key // TODO: versioned like SS has? struct GranuleTenantData : NonCopyable, ReferenceCounted { - TenantName name; TenantMapEntry entry; Reference bstore; Promise bstoreLoaded; GranuleTenantData() {} - GranuleTenantData(TenantName name, TenantMapEntry entry) : name(name), entry(entry) {} + GranuleTenantData(TenantMapEntry entry) : entry(entry) {} void updateBStore(const BlobMetadataDetailsRef& metadata) { if (bstoreLoaded.canBeSet()) { @@ -120,7 +119,7 @@ struct GranuleTenantData : NonCopyable, ReferenceCounted { // TODO: add refreshing struct BGTenantMap { public: - void addTenants(std::vector>); + void addTenants(std::vector>); void removeTenants(std::vector tenantIds); Optional getTenantById(int64_t id); diff --git a/fdbserver/include/fdbserver/ProxyCommitData.actor.h b/fdbserver/include/fdbserver/ProxyCommitData.actor.h index 64125155f2..11c2e20181 100644 --- a/fdbserver/include/fdbserver/ProxyCommitData.actor.h +++ b/fdbserver/include/fdbserver/ProxyCommitData.actor.h @@ -110,7 +110,7 @@ struct ProxyStats { NotifiedVersion* pVersion, NotifiedVersion* pCommittedVersion, int64_t* commitBatchesMemBytesCountPtr, - std::unordered_map* pTenantMap) + std::map* pTenantMap) : cc("ProxyStats", id.toString()), txnCommitIn("TxnCommitIn", cc), txnCommitVersionAssigned("TxnCommitVersionAssigned", cc), txnCommitResolving("TxnCommitResolving", cc), txnCommitResolved("TxnCommitResolved", cc), txnCommitOut("TxnCommitOut", cc), @@ -177,8 +177,8 @@ struct ExpectedIdempotencyIdCountForKey { struct ProxyCommitData { UID dbgid; int64_t commitBatchesMemBytesCount; - std::map tenantNameIndex; - std::unordered_map tenantMap; + std::unordered_map tenantNameIndex; + std::map tenantMap; std::unordered_set tenantsOverStorageQuota; ProxyStats stats; MasterInterface master; diff --git a/fdbserver/include/fdbserver/workloads/MetaclusterConsistency.actor.h b/fdbserver/include/fdbserver/workloads/MetaclusterConsistency.actor.h index fe7c5bb810..f7dcfd964d 100644 --- a/fdbserver/include/fdbserver/workloads/MetaclusterConsistency.actor.h +++ b/fdbserver/include/fdbserver/workloads/MetaclusterConsistency.actor.h @@ -52,10 +52,10 @@ private: KeyBackedRangeResult clusterTenantTuples; KeyBackedRangeResult clusterTenantGroupTuples; - std::map tenantMap; + std::map tenantMap; KeyBackedRangeResult> tenantGroups; - std::map> clusterTenantMap; + std::map> clusterTenantMap; std::map> clusterTenantGroupMap; int64_t tenantCount; @@ -70,7 +70,7 @@ private: ACTOR static Future loadManagementClusterMetadata(MetaclusterConsistencyCheck* self) { state Reference managementTr = self->managementDb->createTransaction(); - state std::vector> tenantList; + state KeyBackedRangeResult> tenantList; loop { try { @@ -99,8 +99,8 @@ private: MetaclusterAPI::ManagementClusterMetadata::tenantMetadata().tenantCount.getD( managementTr, Snapshot::False, 0)) && store(tenantList, - MetaclusterAPI::listTenantsTransaction( - managementTr, ""_sr, "\xff\xff"_sr, metaclusterMaxTenants)) && + MetaclusterAPI::ManagementClusterMetadata::tenantMetadata().tenantMap.getRange( + managementTr, {}, {}, metaclusterMaxTenants)) && store(self->managementMetadata.tenantGroups, MetaclusterAPI::ManagementClusterMetadata::tenantMetadata().tenantGroupMap.getRange( managementTr, {}, {}, metaclusterMaxTenants)) && @@ -113,14 +113,15 @@ private: } } - self->managementMetadata.tenantMap = std::map(tenantList.begin(), tenantList.end()); + self->managementMetadata.tenantMap = + std::map(tenantList.results.begin(), tenantList.results.end()); for (auto t : self->managementMetadata.clusterTenantTuples.results) { ASSERT_EQ(t.size(), 3); TenantName tenantName = t.getString(1); int64_t tenantId = t.getInt(2); - ASSERT_EQ(tenantId, self->managementMetadata.tenantMap[tenantName].id); - self->managementMetadata.clusterTenantMap[t.getString(0)].insert(tenantName); + ASSERT(tenantName == self->managementMetadata.tenantMap[tenantId].tenantName); + self->managementMetadata.clusterTenantMap[t.getString(0)].insert(tenantId); } for (auto t : self->managementMetadata.clusterTenantGroupTuples.results) { @@ -200,13 +201,13 @@ private: // Iterate through all tenants and verify related metadata std::map clusterAllocated; std::set processedTenantGroups; - for (auto [name, entry] : managementMetadata.tenantMap) { + for (auto [tenantId, entry] : managementMetadata.tenantMap) { ASSERT(entry.assignedCluster.present()); // Each tenant should be assigned to the same cluster where it is stored in the cluster tenant index auto clusterItr = managementMetadata.clusterTenantMap.find(entry.assignedCluster.get()); ASSERT(clusterItr != managementMetadata.clusterTenantMap.end()); - ASSERT(clusterItr->second.count(name)); + ASSERT(clusterItr->second.count(tenantId)); if (entry.tenantGroup.present()) { // Count the number of tenant groups allocated in each cluster @@ -216,7 +217,7 @@ private: // The tenant group should be stored in the same cluster where it is stored in the cluster tenant // group index auto clusterTenantGroupItr = managementMetadata.clusterTenantGroupMap.find(entry.assignedCluster.get()); - ASSERT(clusterTenantGroupItr != managementMetadata.clusterTenantMap.end()); + ASSERT(clusterTenantGroupItr != managementMetadata.clusterTenantGroupMap.end()); ASSERT(clusterTenantGroupItr->second.count(entry.tenantGroup.get())); } else { // Track the actual tenant group allocation per cluster (a tenant with no group counts against the @@ -251,7 +252,7 @@ private: state Reference dataTr = dataDb->createTransaction(); state Optional dataClusterRegistration; - state std::vector> dataClusterTenantList; + state KeyBackedRangeResult> dataClusterTenantList; state KeyBackedRangeResult> dataClusterTenantGroupList; state TenantConsistencyCheck tenantConsistencyCheck(dataDb); @@ -262,8 +263,8 @@ private: dataTr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS); wait(store(dataClusterRegistration, MetaclusterMetadata::metaclusterRegistration().get(dataTr)) && store(dataClusterTenantList, - TenantAPI::listTenantsTransaction( - dataTr, ""_sr, "\xff\xff"_sr, CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER + 1)) && + TenantMetadata::tenantMap().getRange( + dataTr, {}, {}, CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER + 1)) && store(dataClusterTenantGroupList, TenantMetadata::tenantGroupMap().getRange( dataTr, {}, {}, CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER + 1))); @@ -274,8 +275,8 @@ private: } } - state std::map dataClusterTenantMap(dataClusterTenantList.begin(), - dataClusterTenantList.end()); + state std::map dataClusterTenantMap(dataClusterTenantList.results.begin(), + dataClusterTenantList.results.end()); state std::map dataClusterTenantGroupMap( dataClusterTenantGroupList.results.begin(), dataClusterTenantGroupList.results.end()); @@ -298,25 +299,20 @@ private: if (metaclusterEntry.tenantGroup.present()) { groupExpectedTenantCounts.try_emplace(metaclusterEntry.tenantGroup.get(), 0); } - if (metaclusterEntry.renamePair.present() && - (metaclusterEntry.tenantState == TenantState::RENAMING_FROM || - metaclusterEntry.tenantState == TenantState::RENAMING_TO)) { - ASSERT(dataClusterTenantMap.count(metaclusterEntry.renamePair.get())); - } else { - ASSERT(metaclusterEntry.tenantState == TenantState::REGISTERING || - metaclusterEntry.tenantState == TenantState::REMOVING); - } + ASSERT(metaclusterEntry.tenantState == TenantState::REGISTERING || + metaclusterEntry.tenantState == TenantState::REMOVING); } else if (metaclusterEntry.tenantGroup.present()) { ++groupExpectedTenantCounts[metaclusterEntry.tenantGroup.get()]; } } } - for (auto [name, entry] : dataClusterTenantMap) { - ASSERT(expectedTenants.count(name)); - TenantMapEntry const& metaclusterEntry = self->managementMetadata.tenantMap[name]; + for (auto [tenantId, entry] : dataClusterTenantMap) { + ASSERT(expectedTenants.count(tenantId)); + TenantMapEntry const& metaclusterEntry = self->managementMetadata.tenantMap[tenantId]; ASSERT(!entry.assignedCluster.present()); ASSERT_EQ(entry.id, metaclusterEntry.id); + ASSERT(entry.tenantName == metaclusterEntry.tenantName); ASSERT_EQ(entry.tenantState, TenantState::READY); if (!self->allowPartialMetaclusterOperations) { diff --git a/fdbserver/include/fdbserver/workloads/TenantConsistency.actor.h b/fdbserver/include/fdbserver/workloads/TenantConsistency.actor.h index 2f037924f9..e1b838f239 100644 --- a/fdbserver/include/fdbserver/workloads/TenantConsistency.actor.h +++ b/fdbserver/include/fdbserver/workloads/TenantConsistency.actor.h @@ -45,16 +45,16 @@ private: struct TenantData { Optional metaclusterRegistration; - std::map tenantMap; - std::map tenantIdIndex; + std::map tenantMap; + std::map tenantNameIndex; int64_t lastTenantId; int64_t tenantCount; std::set tenantTombstones; Optional tombstoneCleanupData; std::map tenantGroupMap; - std::map> tenantGroupIndex; + std::map> tenantGroupIndex; - std::set tenantsInTenantGroupIndex; + std::set tenantsInTenantGroupIndex; ClusterType clusterType; }; @@ -67,8 +67,8 @@ private: ACTOR static Future loadTenantMetadata(TenantConsistencyCheck* self) { state Reference tr = self->db->createTransaction(); - state KeyBackedRangeResult> tenantList; - state KeyBackedRangeResult> tenantIdIndexList; + state KeyBackedRangeResult> tenantList; + state KeyBackedRangeResult> tenantNameIndexList; state KeyBackedRangeResult tenantTombstoneList; state KeyBackedRangeResult> tenantGroupList; state KeyBackedRangeResult tenantGroupTenantTuples; @@ -92,8 +92,8 @@ private: wait( store(tenantList, tenantMetadata->tenantMap.getRange(tr, {}, {}, metaclusterMaxTenants)) && - store(tenantIdIndexList, - tenantMetadata->tenantIdIndex.getRange(tr, {}, {}, metaclusterMaxTenants)) && + store(tenantNameIndexList, + tenantMetadata->tenantNameIndex.getRange(tr, {}, {}, metaclusterMaxTenants)) && store(self->metadata.lastTenantId, tenantMetadata->lastTenantId.getD(tr, Snapshot::False, -1)) && store(self->metadata.tenantCount, tenantMetadata->tenantCount.getD(tr, Snapshot::False, 0)) && store(tenantTombstoneList, @@ -111,11 +111,11 @@ private: ASSERT(!tenantList.more); self->metadata.tenantMap = - std::map(tenantList.results.begin(), tenantList.results.end()); + std::map(tenantList.results.begin(), tenantList.results.end()); - ASSERT(!tenantIdIndexList.more); - self->metadata.tenantIdIndex = - std::map(tenantIdIndexList.results.begin(), tenantIdIndexList.results.end()); + ASSERT(!tenantNameIndexList.more); + self->metadata.tenantNameIndex = + std::map(tenantNameIndexList.results.begin(), tenantNameIndexList.results.end()); ASSERT(!tenantTombstoneList.more); self->metadata.tenantTombstones = @@ -128,11 +128,11 @@ private: for (auto t : tenantGroupTenantTuples.results) { ASSERT_EQ(t.size(), 2); TenantGroupName tenantGroupName = t.getString(0); - TenantName tenantName = t.getString(1); + int64_t tenantId = t.getInt(1); ASSERT(self->metadata.tenantGroupMap.count(tenantGroupName)); - ASSERT(self->metadata.tenantMap.count(tenantName)); - self->metadata.tenantGroupIndex[tenantGroupName].insert(tenantName); - ASSERT(self->metadata.tenantsInTenantGroupIndex.insert(tenantName).second); + ASSERT(self->metadata.tenantMap.count(tenantId)); + self->metadata.tenantGroupIndex[tenantGroupName].insert(tenantId); + ASSERT(self->metadata.tenantsInTenantGroupIndex.insert(tenantId).second); } ASSERT_EQ(self->metadata.tenantGroupIndex.size(), self->metadata.tenantGroupMap.size()); @@ -147,51 +147,46 @@ private: } ASSERT_EQ(metadata.tenantMap.size(), metadata.tenantCount); - ASSERT_EQ(metadata.tenantIdIndex.size(), metadata.tenantCount); + ASSERT_EQ(metadata.tenantNameIndex.size(), metadata.tenantCount); - for (auto [tenantName, tenantMapEntry] : metadata.tenantMap) { + int renameCount = 0; + for (auto [tenantId, tenantMapEntry] : metadata.tenantMap) { + ASSERT_EQ(tenantId, tenantMapEntry.id); if (metadata.clusterType != ClusterType::METACLUSTER_DATA) { - ASSERT_LE(tenantMapEntry.id, metadata.lastTenantId); + ASSERT_LE(tenantId, metadata.lastTenantId); } - ASSERT(metadata.tenantIdIndex[tenantMapEntry.id] == tenantName); + ASSERT_EQ(metadata.tenantNameIndex[tenantMapEntry.tenantName], tenantId); if (tenantMapEntry.tenantGroup.present()) { auto tenantGroupMapItr = metadata.tenantGroupMap.find(tenantMapEntry.tenantGroup.get()); ASSERT(tenantGroupMapItr != metadata.tenantGroupMap.end()); ASSERT(tenantMapEntry.assignedCluster == tenantGroupMapItr->second.assignedCluster); - ASSERT(metadata.tenantGroupIndex[tenantMapEntry.tenantGroup.get()].count(tenantName)); + ASSERT(metadata.tenantGroupIndex[tenantMapEntry.tenantGroup.get()].count(tenantId)); } else { - ASSERT(!metadata.tenantsInTenantGroupIndex.count(tenantName)); + ASSERT(!metadata.tenantsInTenantGroupIndex.count(tenantId)); } if (metadata.clusterType == ClusterType::METACLUSTER_MANAGEMENT) { ASSERT(tenantMapEntry.assignedCluster.present()); - // If the rename pair is present, it should be in the map and match our current entry - if (tenantMapEntry.renamePair.present()) { - auto pairMapEntry = metadata.tenantMap[tenantMapEntry.renamePair.get()]; - ASSERT_EQ(pairMapEntry.id, tenantMapEntry.id); - ASSERT(pairMapEntry.prefix == tenantMapEntry.prefix); - ASSERT_EQ(pairMapEntry.configurationSequenceNum, tenantMapEntry.configurationSequenceNum); - ASSERT(pairMapEntry.assignedCluster.present()); - ASSERT(pairMapEntry.assignedCluster.get() == tenantMapEntry.assignedCluster.get()); - ASSERT(pairMapEntry.renamePair.present()); - ASSERT(pairMapEntry.renamePair.get() == tenantName); - if (tenantMapEntry.tenantState == TenantState::RENAMING_FROM) { - ASSERT_EQ(pairMapEntry.tenantState, TenantState::RENAMING_TO); - } else if (tenantMapEntry.tenantState == TenantState::RENAMING_TO) { - ASSERT_EQ(pairMapEntry.tenantState, TenantState::RENAMING_FROM); - } else if (tenantMapEntry.tenantState == TenantState::REMOVING) { - ASSERT_EQ(pairMapEntry.tenantState, TenantState::REMOVING); - } else { - ASSERT(false); // Entry in an invalid state if we have a rename pair - } + if (tenantMapEntry.renameDestination.present()) { + ASSERT(tenantMapEntry.tenantState == TenantState::RENAMING || + tenantMapEntry.tenantState == TenantState::REMOVING); + + auto nameIndexItr = metadata.tenantNameIndex.find(tenantMapEntry.renameDestination.get()); + ASSERT(nameIndexItr != metadata.tenantNameIndex.end()); + ASSERT_EQ(nameIndexItr->second, tenantMapEntry.id); + ++renameCount; + } else { + ASSERT_NE(tenantMapEntry.tenantState, TenantState::RENAMING); } } else { ASSERT_EQ(tenantMapEntry.tenantState, TenantState::READY); ASSERT(!tenantMapEntry.assignedCluster.present()); - ASSERT(!tenantMapEntry.renamePair.present()); + ASSERT(!tenantMapEntry.renameDestination.present()); } } + + ASSERT_EQ(metadata.tenantMap.size() + renameCount, metadata.tenantNameIndex.size()); } // Check that the tenant tombstones are properly cleaned up and only present on a metacluster data cluster diff --git a/fdbserver/storageserver.actor.cpp b/fdbserver/storageserver.actor.cpp index 587e361743..239f3e6580 100644 --- a/fdbserver/storageserver.actor.cpp +++ b/fdbserver/storageserver.actor.cpp @@ -11233,7 +11233,7 @@ ACTOR Future initTenantMap(StorageServer* self) { state Version version = wait(tr->getReadVersion()); // This limits the number of tenants, but eventually we shouldn't need to do this at all // when SSs store only the local tenants - KeyBackedRangeResult> entries = + KeyBackedRangeResult> entries = wait(TenantMetadata::tenantMap().getRange(tr, {}, {}, CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER + 1)); ASSERT(entries.results.size() <= CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER && !entries.more); @@ -11241,8 +11241,8 @@ ACTOR Future initTenantMap(StorageServer* self) { .detail("Version", version) .detail("NumTenants", entries.results.size()); - for (auto entry : entries.results) { - self->insertTenant(entry.second.prefix, entry.first, version, false); + for (auto const& [_, entry] : entries.results) { + self->insertTenant(entry.prefix, entry.tenantName, version, false); } break; } catch (Error& e) { diff --git a/fdbserver/workloads/BlobGranuleCorrectnessWorkload.actor.cpp b/fdbserver/workloads/BlobGranuleCorrectnessWorkload.actor.cpp index 08f84c0fdf..183b178228 100644 --- a/fdbserver/workloads/BlobGranuleCorrectnessWorkload.actor.cpp +++ b/fdbserver/workloads/BlobGranuleCorrectnessWorkload.actor.cpp @@ -128,7 +128,10 @@ struct ThreadData : ReferenceCounted, NonCopyable { } } - void openTenant(Database const& cx) { tenant = makeReference(cx, tenantName); } + Future openTenant(Database const& cx) { + tenant = makeReference(cx, tenantName); + return tenant->ready(); + } // TODO could make keys variable length? Key getKey(uint32_t key, uint32_t id) { @@ -282,17 +285,17 @@ struct BlobGranuleCorrectnessWorkload : TestWorkload { } state int directoryIdx = 0; - state std::vector> tenants; + state std::vector> tenants; state BGTenantMap tenantData(self->dbInfo); state Reference data; for (; directoryIdx < self->directories.size(); directoryIdx++) { // Set up the blob range first - TenantMapEntry tenantEntry = wait(self->setUpTenant(cx, self->directories[directoryIdx]->tenantName)); - self->directories[directoryIdx]->openTenant(cx); + state TenantMapEntry tenantEntry = wait(self->setUpTenant(cx, self->directories[directoryIdx]->tenantName)); + wait(self->directories[directoryIdx]->openTenant(cx)); self->directories[directoryIdx]->tenantEntry = tenantEntry; self->directories[directoryIdx]->directoryRange = KeyRangeRef(tenantEntry.prefix, tenantEntry.prefix.withSuffix(normalKeys.end)); - tenants.push_back({ self->directories[directoryIdx]->tenant->name.get(), tenantEntry }); + tenants.push_back({ self->directories[directoryIdx]->tenant->id(), tenantEntry }); bool _success = wait(cx->blobbifyRange(self->directories[directoryIdx]->directoryRange)); ASSERT(_success); } diff --git a/fdbserver/workloads/MetaclusterManagementWorkload.actor.cpp b/fdbserver/workloads/MetaclusterManagementWorkload.actor.cpp index 61c5e23be8..4fc3b4e4d1 100644 --- a/fdbserver/workloads/MetaclusterManagementWorkload.actor.cpp +++ b/fdbserver/workloads/MetaclusterManagementWorkload.actor.cpp @@ -424,6 +424,7 @@ struct MetaclusterManagementWorkload : TestWorkload { ACTOR static Future createTenant(MetaclusterManagementWorkload* self) { state TenantName tenant = self->chooseTenantName(); state Optional tenantGroup = self->chooseTenantGroup(); + state AssignClusterAutomatically assignClusterAutomatically(deterministicRandom()->coinflip()); auto itr = self->createdTenants.find(tenant); state bool exists = itr != self->createdTenants.end(); @@ -431,24 +432,24 @@ struct MetaclusterManagementWorkload : TestWorkload { state bool hasCapacity = tenantGroupExists || self->ungroupedTenants.size() + self->tenantGroups.size() < self->totalTenantGroupCapacity; state bool retried = false; - state bool preferAssignedCluster = deterministicRandom()->coinflip(); // Choose between two preferred clusters because if we get a partial completion and // retry, we want the operation to eventually succeed instead of having a chance of // never re-visiting the original preferred cluster. state std::pair preferredClusters; state Optional originalPreferredCluster; - if (preferAssignedCluster) { + if (!assignClusterAutomatically) { preferredClusters.first = self->chooseClusterName(); preferredClusters.second = self->chooseClusterName(); } state TenantMapEntry tenantMapEntry; + tenantMapEntry.tenantName = tenant; tenantMapEntry.tenantGroup = tenantGroup; try { loop { try { - if (preferAssignedCluster && (!retried || deterministicRandom()->coinflip())) { + if (!assignClusterAutomatically && (!retried || deterministicRandom()->coinflip())) { tenantMapEntry.assignedCluster = deterministicRandom()->coinflip() ? preferredClusters.first : preferredClusters.second; if (!originalPreferredCluster.present()) { @@ -456,7 +457,7 @@ struct MetaclusterManagementWorkload : TestWorkload { } } Future createFuture = - MetaclusterAPI::createTenant(self->managementDb, tenant, tenantMapEntry); + MetaclusterAPI::createTenant(self->managementDb, tenantMapEntry, assignClusterAutomatically); Optional result = wait(timeout(createFuture, deterministicRandom()->randomInt(1, 30))); if (result.present()) { break; @@ -470,7 +471,7 @@ struct MetaclusterManagementWorkload : TestWorkload { ASSERT(entry.present()); tenantMapEntry = entry.get(); break; - } else if (preferAssignedCluster && retried && + } else if (!assignClusterAutomatically && retried && originalPreferredCluster.get() != tenantMapEntry.assignedCluster.get() && (e.code() == error_code_cluster_no_capacity || e.code() == error_code_cluster_not_found || @@ -503,7 +504,7 @@ struct MetaclusterManagementWorkload : TestWorkload { } auto assignedCluster = self->dataDbs.find(entry.assignedCluster.get()); - ASSERT(!preferAssignedCluster || tenantMapEntry.assignedCluster.get() == assignedCluster->first); + ASSERT(assignClusterAutomatically || tenantMapEntry.assignedCluster.get() == assignedCluster->first); ASSERT(assignedCluster != self->dataDbs.end()); ASSERT(assignedCluster->second.tenants.insert(tenant).second); @@ -526,10 +527,10 @@ struct MetaclusterManagementWorkload : TestWorkload { ASSERT(!hasCapacity && !exists); return Void(); } else if (e.code() == error_code_cluster_no_capacity) { - ASSERT(preferAssignedCluster); + ASSERT(!assignClusterAutomatically); return Void(); } else if (e.code() == error_code_cluster_not_found) { - ASSERT(preferAssignedCluster); + ASSERT(!assignClusterAutomatically); return Void(); } else if (e.code() == error_code_invalid_tenant_configuration) { ASSERT(tenantGroup.present()); diff --git a/fdbserver/workloads/TenantEntryCacheWorkload.actor.cpp b/fdbserver/workloads/TenantEntryCacheWorkload.actor.cpp index 66a577d08d..62970629a8 100644 --- a/fdbserver/workloads/TenantEntryCacheWorkload.actor.cpp +++ b/fdbserver/workloads/TenantEntryCacheWorkload.actor.cpp @@ -34,9 +34,8 @@ #include "flow/actorcompiler.h" // This must be the last #include. namespace { -TenantEntryCachePayload createPayload(const TenantName& name, const TenantMapEntry& entry) { +TenantEntryCachePayload createPayload(const TenantMapEntry& entry) { TenantEntryCachePayload payload; - payload.name = name; payload.entry = entry; payload.payload = entry.id; @@ -66,16 +65,16 @@ struct TenantEntryCacheWorkload : TestWorkload { ASSERT_EQ(left.get().payload, right.id); } - ACTOR static Future compareContents(std::vector>* tenants, + ACTOR static Future compareContents(std::vector* tenants, Reference> cache) { state int i; for (i = 0; i < tenants->size(); i++) { if (deterministicRandom()->coinflip()) { - Optional> e = wait(cache->getById(tenants->at(i).second.id)); - compareTenants(e, tenants->at(i).second); + Optional> e = wait(cache->getById(tenants->at(i).id)); + compareTenants(e, tenants->at(i)); } else { - Optional> e = wait(cache->getByName(tenants->at(i).first)); - compareTenants(e, tenants->at(i).second); + Optional> e = wait(cache->getByName(tenants->at(i).tenantName)); + compareTenants(e, tenants->at(i)); } } @@ -104,7 +103,7 @@ struct TenantEntryCacheWorkload : TestWorkload { ACTOR static Future testCreateTenantsAndLookup(Database cx, TenantEntryCacheWorkload* self, - std::vector>* tenantList, + std::vector* tenantList, TenantEntryCacheRefreshMode refreshMode) { state Reference> cache = makeReference>( cx, deterministicRandom()->randomUniqueID(), createPayload, refreshMode); @@ -128,9 +127,9 @@ struct TenantEntryCacheWorkload : TestWorkload { continue; } - Optional entry = wait(TenantAPI::createTenant(cx.getReference(), StringRef(name))); + Optional entry = wait(TenantAPI::createTenant(cx.getReference(), name)); ASSERT(entry.present()); - tenantList->emplace_back(std::make_pair(name, entry.get())); + tenantList->emplace_back(entry.get()); tenantNames.emplace(name); i++; } @@ -143,7 +142,7 @@ struct TenantEntryCacheWorkload : TestWorkload { ACTOR static Future testTenantInsert(Database cx, TenantEntryCacheWorkload* self, - std::vector>* tenantList, + std::vector* tenantList, TenantEntryCacheRefreshMode refreshMode) { state Reference> cache = makeReference>( cx, deterministicRandom()->randomUniqueID(), createPayload, refreshMode); @@ -157,42 +156,44 @@ struct TenantEntryCacheWorkload : TestWorkload { ASSERT_EQ(cache->numRefreshByInit(), 1); ASSERT_GE(cache->numCacheRefreshes(), 1); - state std::pair p = tenantList->at(0); + state TenantMapEntry p = tenantList->at(0); state Optional> entry; // Tenant rename - p.first = TenantName(format("%s%08d", - self->localTenantNamePrefix.toString().c_str(), - deterministicRandom()->randomInt(self->maxTenants + 100, self->maxTenants + 200))); + p.tenantName = + TenantName(format("%s%08d", + self->localTenantNamePrefix.toString().c_str(), + deterministicRandom()->randomInt(self->maxTenants + 100, self->maxTenants + 200))); cache->put(p); - Optional> e = wait(cache->getByName(p.first)); + Optional> e = wait(cache->getByName(p.tenantName)); entry = e; - compareTenants(entry, p.second); + compareTenants(entry, p); // Tenant delete & recreate - p.second.id = p.second.id + deterministicRandom()->randomInt(self->maxTenants + 500, self->maxTenants + 700); + p.id = p.id + deterministicRandom()->randomInt(self->maxTenants + 500, self->maxTenants + 700); cache->put(p); - Optional> e1 = wait(cache->getById(p.second.id)); + Optional> e1 = wait(cache->getById(p.id)); entry = e1; - compareTenants(entry, p.second); - ASSERT_EQ(p.first.contents().toString().compare(entry.get().name.contents().toString()), 0); + compareTenants(entry, p); + ASSERT_EQ(p.tenantName.compare(entry.get().entry.tenantName), 0); // Delete a tenant and rename an existing TenantEntry to reuse the name of deleted tenant - state std::pair p1 = tenantList->back(); + state TenantMapEntry p1 = tenantList->back(); tenantList->pop_back(); - wait(TenantAPI::deleteTenant(cx.getReference(), p1.first)); - cache->put(std::make_pair(p1.first, p.second)); - Optional> e2 = wait(cache->getById(p.second.id)); + wait(TenantAPI::deleteTenant(cx.getReference(), p1.tenantName)); + p.tenantName = p1.tenantName; + cache->put(p); + Optional> e2 = wait(cache->getById(p.id)); entry = e2; - compareTenants(entry, p.second); - ASSERT_EQ(p1.first.contents().toString().compare(entry.get().name.contents().toString()), 0); + compareTenants(entry, p); + ASSERT_EQ(p1.tenantName.compare(entry.get().entry.tenantName), 0); TraceEvent("TestTenantInsertEnd"); return Void(); } ACTOR static Future testCacheReload(Database cx, - std::vector>* tenantList, + std::vector* tenantList, TenantEntryCacheRefreshMode refreshMode) { state Reference> cache = makeReference>( cx, deterministicRandom()->randomUniqueID(), createPayload, refreshMode); @@ -270,8 +271,7 @@ struct TenantEntryCacheWorkload : TestWorkload { return Void(); } - ACTOR static Future tenantEntryRemove(Database cx, - std::vector>* tenantList) { + ACTOR static Future tenantEntryRemove(Database cx, std::vector* tenantList) { state Reference> cache = makeReference>( cx, deterministicRandom()->randomUniqueID(), createPayload, TenantEntryCacheRefreshMode::NONE); @@ -281,35 +281,34 @@ struct TenantEntryCacheWorkload : TestWorkload { // Remove an entry from the cache state int idx = deterministicRandom()->randomInt(0, tenantList->size()); - Optional> entry = wait(cache->getByName(tenantList->at(idx).first)); + Optional> entry = wait(cache->getByName(tenantList->at(idx).tenantName)); ASSERT(entry.present()); TraceEvent("TestTenantEntryRemoveStart") - .detail("Id", tenantList->at(idx).second.id) - .detail("Name", tenantList->at(idx).first) - .detail("Prefix", tenantList->at(idx).second.prefix); + .detail("Id", tenantList->at(idx).id) + .detail("Name", tenantList->at(idx).tenantName) + .detail("Prefix", tenantList->at(idx).prefix); - wait(TenantAPI::deleteTenant(cx.getReference(), tenantList->at(idx).first)); + wait(TenantAPI::deleteTenant(cx.getReference(), tenantList->at(idx).tenantName)); if (deterministicRandom()->coinflip()) { - wait(cache->removeEntryById(tenantList->at(idx).second.id)); + wait(cache->removeEntryById(tenantList->at(idx).id)); } else if (deterministicRandom()->coinflip()) { - wait(cache->removeEntryByPrefix(tenantList->at(idx).second.prefix)); + wait(cache->removeEntryByPrefix(tenantList->at(idx).prefix)); } else { - wait(cache->removeEntryByName(tenantList->at(idx).first)); + wait(cache->removeEntryByName(tenantList->at(idx).tenantName)); } - state Optional> e = wait(cache->getById(tenantList->at(idx).second.id)); + state Optional> e = wait(cache->getById(tenantList->at(idx).id)); ASSERT(!e.present()); - state Optional> e1 = - wait(cache->getByPrefix(tenantList->at(idx).second.prefix)); + state Optional> e1 = wait(cache->getByPrefix(tenantList->at(idx).prefix)); ASSERT(!e1.present()); - state Optional> e2 = wait(cache->getByName(tenantList->at(idx).first)); + state Optional> e2 = wait(cache->getByName(tenantList->at(idx).tenantName)); ASSERT(!e2.present()); // Ensure remove-entry is an idempotent operation - cache->removeEntryByName(tenantList->at(idx).first); - Optional> e3 = wait(cache->getById(tenantList->at(idx).second.id)); + cache->removeEntryByName(tenantList->at(idx).tenantName); + Optional> e3 = wait(cache->getById(tenantList->at(idx).id)); ASSERT(!e3.present()); return Void(); @@ -365,7 +364,7 @@ struct TenantEntryCacheWorkload : TestWorkload { } ACTOR Future _start(Database cx, TenantEntryCacheWorkload* self) { - state std::vector> tenantList; + state std::vector tenantList; state TenantEntryCacheRefreshMode refreshMode; if (deterministicRandom()->coinflip()) { refreshMode = TenantEntryCacheRefreshMode::PERIODIC_TASK; diff --git a/fdbserver/workloads/TenantManagementConcurrencyWorkload.actor.cpp b/fdbserver/workloads/TenantManagementConcurrencyWorkload.actor.cpp index 4a4133c4ea..f0626436b0 100644 --- a/fdbserver/workloads/TenantManagementConcurrencyWorkload.actor.cpp +++ b/fdbserver/workloads/TenantManagementConcurrencyWorkload.actor.cpp @@ -171,13 +171,15 @@ struct TenantManagementConcurrencyWorkload : TestWorkload { ACTOR static Future createTenant(TenantManagementConcurrencyWorkload* self) { state TenantName tenant = self->chooseTenantName(); state TenantMapEntry entry; + entry.tenantName = tenant; entry.tenantGroup = self->chooseTenantGroup(); try { loop { Future createFuture = - self->useMetacluster ? MetaclusterAPI::createTenant(self->mvDb, tenant, entry) - : success(TenantAPI::createTenant(self->dataDb.getReference(), tenant, entry)); + self->useMetacluster + ? MetaclusterAPI::createTenant(self->mvDb, entry, AssignClusterAutomatically::True) + : success(TenantAPI::createTenant(self->dataDb.getReference(), tenant, entry)); Optional result = wait(timeout(createFuture, 30)); if (result.present()) { break; @@ -236,7 +238,7 @@ struct TenantManagementConcurrencyWorkload : TestWorkload { for (auto param : configParams) { updatedEntry.configure(param.first, param.second); } - wait(TenantAPI::configureTenantTransaction(tr, tenant, entry, updatedEntry)); + wait(TenantAPI::configureTenantTransaction(tr, entry, updatedEntry)); wait(buggifiedCommit(tr, BUGGIFY_WITH_PROB(0.1))); break; } catch (Error& e) { diff --git a/fdbserver/workloads/TenantManagementWorkload.actor.cpp b/fdbserver/workloads/TenantManagementWorkload.actor.cpp index 041e2af2ce..f60258f3c0 100644 --- a/fdbserver/workloads/TenantManagementWorkload.actor.cpp +++ b/fdbserver/workloads/TenantManagementWorkload.actor.cpp @@ -26,6 +26,7 @@ #include "fdbclient/FDBTypes.h" #include "fdbclient/GenericManagementAPI.actor.h" #include "fdbclient/KeyBackedTypes.h" +#include "fdbclient/KeyRangeMap.h" #include "fdbclient/MetaclusterManagement.actor.h" #include "fdbclient/ReadYourWrites.h" #include "fdbclient/RunRYWTransaction.actor.h" @@ -386,7 +387,7 @@ struct TenantManagementWorkload : TestWorkload { std::vector> createFutures; for (auto [tenant, entry] : tenantsToCreate) { entry.setId(nextId++); - createFutures.push_back(success(TenantAPI::createTenantTransaction(tr, tenant, entry))); + createFutures.push_back(success(TenantAPI::createTenantTransaction(tr, entry))); } TenantMetadata::lastTenantId().set(tr, nextId - 1); wait(waitForAll(createFutures)); @@ -397,7 +398,7 @@ struct TenantManagementWorkload : TestWorkload { tenantsToCreate.begin()->second.assignedCluster = self->dataClusterName; } wait(MetaclusterAPI::createTenant( - self->mvDb, tenantsToCreate.begin()->first, tenantsToCreate.begin()->second)); + self->mvDb, tenantsToCreate.begin()->second, AssignClusterAutomatically::True)); } return Void(); @@ -431,6 +432,7 @@ struct TenantManagementWorkload : TestWorkload { } TenantMapEntry entry; + entry.tenantName = tenant; entry.tenantGroup = self->chooseTenantGroup(true); if (self->createdTenants.count(tenant)) { @@ -704,10 +706,9 @@ struct TenantManagementWorkload : TestWorkload { ACTOR static Future deleteTenantImpl(Reference tr, TenantName beginTenant, Optional endTenant, - std::vector tenants, + std::map tenants, OperationType operationType, TenantManagementWorkload* self) { - state int tenantIndex; if (operationType == OperationType::SPECIAL_KEYS) { tr->setOption(FDBTransactionOptions::SPECIAL_KEY_SPACE_ENABLE_WRITES); Key key = self->specialKeysTenantMapPrefix.withSuffix(beginTenant); @@ -723,8 +724,10 @@ struct TenantManagementWorkload : TestWorkload { } else if (operationType == OperationType::MANAGEMENT_TRANSACTION) { tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); std::vector> deleteFutures; - for (tenantIndex = 0; tenantIndex != tenants.size(); ++tenantIndex) { - deleteFutures.push_back(TenantAPI::deleteTenantTransaction(tr, tenants[tenantIndex])); + for (auto const& [name, id] : tenants) { + if (id != TenantInfo::INVALID_TENANT) { + deleteFutures.push_back(TenantAPI::deleteTenantTransaction(tr, id)); + } } wait(waitForAll(deleteFutures)); @@ -782,38 +785,38 @@ struct TenantManagementWorkload : TestWorkload { state bool anyExists = alreadyExists; // Collect a list of all tenants that we expect should be deleted by this operation - state std::vector tenants; + state std::map tenants; if (!endTenant.present()) { - tenants.push_back(beginTenant); + tenants[beginTenant] = anyExists ? itr->second.tenant->id() : TenantInfo::INVALID_TENANT; } else if (endTenant.present()) { for (auto itr = self->createdTenants.lower_bound(beginTenant); itr != self->createdTenants.end() && itr->first < endTenant.get(); ++itr) { - tenants.push_back(itr->first); + tenants[itr->first] = itr->second.tenant->id(); anyExists = true; } } // Check whether each tenant is empty. - state int tenantIndex; + state std::map::iterator tenantItr; state std::vector>>> watchFutures; try { if (alreadyExists || endTenant.present()) { - for (tenantIndex = 0; tenantIndex < tenants.size(); ++tenantIndex) { + for (tenantItr = tenants.begin(); tenantItr != tenants.end(); ++tenantItr) { // For most tenants, we will delete the contents and make them empty if (deterministicRandom()->random01() < 0.9) { - wait(clearTenantData(self, tenants[tenantIndex])); + wait(clearTenantData(self, tenantItr->first)); // watch the tenant to be deleted if (watchTenantCheck) { watchFutures.emplace_back( - tenants[tenantIndex], - errorOr(watchTenant(self, self->createdTenants[tenants[tenantIndex]].tenant))); + tenantItr->first, + errorOr(watchTenant(self, self->createdTenants[tenantItr->first].tenant))); } } // Otherwise, we will just report the current emptiness of the tenant else { - auto itr = self->createdTenants.find(tenants[tenantIndex]); + auto itr = self->createdTenants.find(tenantItr->first); ASSERT(itr != self->createdTenants.end()); isEmpty = isEmpty && itr->second.empty; } @@ -881,7 +884,7 @@ struct TenantManagementWorkload : TestWorkload { if (!tenants.empty()) { // Check the state of the first deleted tenant Optional resultEntry = - wait(self->tryGetTenant(*tenants.begin(), operationType)); + wait(self->tryGetTenant(tenants.begin()->first, operationType)); if (!resultEntry.present()) { alreadyExists = false; @@ -893,9 +896,8 @@ struct TenantManagementWorkload : TestWorkload { } } - // The management transaction operation is a no-op if there are no tenants to delete in a range - // delete - if (tenants.size() == 0 && operationType == OperationType::MANAGEMENT_TRANSACTION) { + // The management transaction operation is a no-op if there are no tenants to delete + if (!anyExists && operationType == OperationType::MANAGEMENT_TRANSACTION) { return Void(); } @@ -928,8 +930,8 @@ struct TenantManagementWorkload : TestWorkload { } // Update our local state to remove the deleted tenants - for (auto tenant : tenants) { - auto itr = self->createdTenants.find(tenant); + for (auto const& [name, id] : tenants) { + auto itr = self->createdTenants.find(name); ASSERT(itr != self->createdTenants.end()); // If the tenant group has no tenants remaining, stop tracking it @@ -941,7 +943,7 @@ struct TenantManagementWorkload : TestWorkload { } } - self->createdTenants.erase(tenant); + self->createdTenants.erase(name); } // check for watch result @@ -1677,7 +1679,7 @@ struct TenantManagementWorkload : TestWorkload { ASSERT(tenantGroups.size() <= limit); - // Compare the resulting tenant list to the list we expected to get + // Compare the resulting tenant group list to the list we expected to get auto localItr = self->createdTenantGroups.lower_bound(beginTenantGroup); auto tenantMapItr = tenantGroups.begin(); for (; tenantMapItr != tenantGroups.end(); ++tenantMapItr, ++localItr) {