From eef2dfd84b3e6153d564e383be183bc848416efc Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Thu, 2 Jun 2022 15:21:27 -0700 Subject: [PATCH 1/3] Update test to exercise multiple tenant creation --- .../workloads/TenantManagement.actor.cpp | 125 +++++++++++------- 1 file changed, 79 insertions(+), 46 deletions(-) diff --git a/fdbserver/workloads/TenantManagement.actor.cpp b/fdbserver/workloads/TenantManagement.actor.cpp index 2d696fc745..75d6da06c7 100644 --- a/fdbserver/workloads/TenantManagement.actor.cpp +++ b/fdbserver/workloads/TenantManagement.actor.cpp @@ -58,7 +58,7 @@ struct TenantManagementWorkload : TestWorkload { enum class OperationType { SPECIAL_KEYS, MANAGEMENT_DATABASE, MANAGEMENT_TRANSACTION }; static OperationType randomOperationType() { - int randomNum = deterministicRandom()->randomInt(0, 3); + int randomNum = deterministicRandom()->randomInt(1, 3); if (randomNum == 0) { return OperationType::SPECIAL_KEYS; } else if (randomNum == 1) { @@ -131,88 +131,121 @@ struct TenantManagementWorkload : TestWorkload { } ACTOR Future createTenant(Database cx, TenantManagementWorkload* self) { - state TenantName tenant = self->chooseTenantName(true); - state bool alreadyExists = self->createdTenants.count(tenant); state OperationType operationType = TenantManagementWorkload::randomOperationType(); + int numTenants = 1; + + // For transaction-based operations, test creating multiple tenants in the same transaction + if (operationType == OperationType::SPECIAL_KEYS || operationType == OperationType::MANAGEMENT_TRANSACTION) { + numTenants = deterministicRandom()->randomInt(1, 5); + } + + state bool alreadyExists = false; + state bool hasSystemTenant = false; + + state std::set tenantsToCreate; + for (int i = 0; i < numTenants; ++i) { + TenantName tenant = self->chooseTenantName(true); + tenantsToCreate.insert(tenant); + + alreadyExists = alreadyExists || self->createdTenants.count(tenant); + hasSystemTenant = hasSystemTenant || tenant.startsWith("\xff"_sr); + } + state Reference tr = makeReference(cx); loop { try { if (operationType == OperationType::SPECIAL_KEYS) { tr->setOption(FDBTransactionOptions::SPECIAL_KEY_SPACE_ENABLE_WRITES); - Key key = self->specialKeysTenantMapPrefix.withSuffix(tenant); - tr->set(key, ""_sr); + for (auto tenant : tenantsToCreate) { + tr->set(self->specialKeysTenantMapPrefix.withSuffix(tenant), ""_sr); + } wait(tr->commit()); } else if (operationType == OperationType::MANAGEMENT_DATABASE) { - wait(ManagementAPI::createTenant(cx.getReference(), tenant)); + ASSERT(tenantsToCreate.size() == 1); + wait(ManagementAPI::createTenant(cx.getReference(), *tenantsToCreate.begin())); } else { tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); - Optional _ = wait(ManagementAPI::createTenantTransaction(tr, tenant)); + for (auto tenant : tenantsToCreate) { + Optional _ = wait(ManagementAPI::createTenantTransaction(tr, tenant)); + } wait(tr->commit()); } - if (operationType != OperationType::MANAGEMENT_DATABASE && alreadyExists) { - return Void(); + if (operationType == OperationType::MANAGEMENT_DATABASE) { + ASSERT(!alreadyExists); } - ASSERT(!alreadyExists); - ASSERT(!tenant.startsWith("\xff"_sr)); + ASSERT(!hasSystemTenant); - state Optional entry = wait(ManagementAPI::tryGetTenant(cx.getReference(), tenant)); - ASSERT(entry.present()); - ASSERT(entry.get().id > self->maxId); - ASSERT(entry.get().prefix.startsWith(self->tenantSubspace)); + state std::set::iterator tenantItr; + for (tenantItr = tenantsToCreate.begin(); tenantItr != tenantsToCreate.end(); ++tenantItr) { + if (self->createdTenants.count(*tenantItr)) { + continue; + } - self->maxId = entry.get().id; - self->createdTenants[tenant] = TenantState(entry.get().id, true); + state Optional entry = + wait(ManagementAPI::tryGetTenant(cx.getReference(), *tenantItr)); + ASSERT(entry.present()); + ASSERT(entry.get().id > self->maxId); + ASSERT(entry.get().prefix.startsWith(self->tenantSubspace)); - state bool insertData = deterministicRandom()->random01() < 0.5; - if (insertData) { - state Transaction insertTr(cx, tenant); - loop { - try { - insertTr.set(self->keyName, tenant); - wait(insertTr.commit()); - break; - } catch (Error& e) { - wait(insertTr.onError(e)); + self->maxId = entry.get().id; + self->createdTenants[*tenantItr] = TenantState(entry.get().id, true); + + state bool insertData = deterministicRandom()->random01() < 0.5; + if (insertData) { + state Transaction insertTr(cx, *tenantItr); + loop { + try { + insertTr.set(self->keyName, *tenantItr); + wait(insertTr.commit()); + break; + } catch (Error& e) { + wait(insertTr.onError(e)); + } + } + + self->createdTenants[*tenantItr].empty = false; + + state Transaction checkTr(cx); + loop { + try { + checkTr.setOption(FDBTransactionOptions::RAW_ACCESS); + Optional val = wait(checkTr.get(self->keyName.withPrefix(entry.get().prefix))); + ASSERT(val.present()); + ASSERT(val.get() == *tenantItr); + break; + } catch (Error& e) { + wait(checkTr.onError(e)); + } } } - self->createdTenants[tenant].empty = false; - - state Transaction checkTr(cx); - loop { - try { - checkTr.setOption(FDBTransactionOptions::RAW_ACCESS); - Optional val = wait(checkTr.get(self->keyName.withPrefix(entry.get().prefix))); - ASSERT(val.present()); - ASSERT(val.get() == tenant); - break; - } catch (Error& e) { - wait(checkTr.onError(e)); - } - } + wait(self->checkTenant(cx, self, *tenantItr, self->createdTenants[*tenantItr])); } - - wait(self->checkTenant(cx, self, tenant, self->createdTenants[tenant])); return Void(); } catch (Error& e) { if (e.code() == error_code_invalid_tenant_name) { - ASSERT(tenant.startsWith("\xff"_sr)); + ASSERT(hasSystemTenant); return Void(); } else if (operationType == OperationType::MANAGEMENT_DATABASE) { if (e.code() == error_code_tenant_already_exists) { ASSERT(alreadyExists && operationType == OperationType::MANAGEMENT_DATABASE); } else { - TraceEvent(SevError, "CreateTenantFailure").error(e).detail("TenantName", tenant); + ASSERT(tenantsToCreate.size() == 1); + TraceEvent(SevError, "CreateTenantFailure") + .error(e) + .detail("TenantName", *tenantsToCreate.begin()); } return Void(); } else { try { wait(tr->onError(e)); } catch (Error& e) { - TraceEvent(SevError, "CreateTenantFailure").error(e).detail("TenantName", tenant); + for (auto tenant : tenantsToCreate) { + TraceEvent(SevError, "CreateTenantFailure").error(e).detail("TenantName", tenant); + } return Void(); } } From 90625ba20d5aa9e2501d477bc8513b57adc80ce4 Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Fri, 3 Jun 2022 15:09:49 -0700 Subject: [PATCH 2/3] Update the create tenant transaction to take the ID as a parameter. Generate unique IDs for multiple creations in the same transaction. Don't set lock aware options inside the tenant transaction code. --- fdbclient/GenericManagementAPI.actor.h | 33 +++++++++---------- fdbclient/SpecialKeySpace.actor.cpp | 27 +++++++++++++-- fdbserver/tester.actor.cpp | 2 +- .../BlobGranuleCorrectnessWorkload.actor.cpp | 26 +++------------ .../workloads/FuzzApiCorrectness.actor.cpp | 2 +- .../workloads/TenantManagement.actor.cpp | 14 ++++++-- 6 files changed, 57 insertions(+), 47 deletions(-) diff --git a/fdbclient/GenericManagementAPI.actor.h b/fdbclient/GenericManagementAPI.actor.h index d1128f79b3..fd9ca36101 100644 --- a/fdbclient/GenericManagementAPI.actor.h +++ b/fdbclient/GenericManagementAPI.actor.h @@ -671,7 +671,6 @@ Future> tryGetTenantTransaction(Transaction tr, TenantN state Key tenantMapKey = name.withPrefix(tenantMapPrefix); tr->setOption(FDBTransactionOptions::RAW_ACCESS); - tr->setOption(FDBTransactionOptions::READ_LOCK_AWARE); state typename transaction_future_type>::type tenantFuture = tr->get(tenantMapKey); Optional val = wait(safeThreadFutureToFuture(tenantFuture)); @@ -714,8 +713,10 @@ Future getTenant(Reference db, TenantName name) { } // Creates a tenant with the given name. If the tenant already exists, an empty optional will be returned. +// The caller must enforce that the tenant ID be unique from all current and past tenants, and it must also be unique +// from all other tenants created in the same transaction. ACTOR template -Future> createTenantTransaction(Transaction tr, TenantNameRef name) { +Future> createTenantTransaction(Transaction tr, TenantNameRef name, int64_t tenantId) { state Key tenantMapKey = name.withPrefix(tenantMapPrefix); if (name.startsWith("\xff"_sr)) { @@ -723,12 +724,10 @@ Future> createTenantTransaction(Transaction tr, TenantN } tr->setOption(FDBTransactionOptions::RAW_ACCESS); - tr->setOption(FDBTransactionOptions::LOCK_AWARE); state Future> tenantEntryFuture = tryGetTenantTransaction(tr, name); state typename transaction_future_type>::type tenantDataPrefixFuture = tr->get(tenantDataPrefixKey); - state typename transaction_future_type>::type lastIdFuture = tr->get(tenantLastIdKey); state typename transaction_future_type>::type tenantModeFuture = tr->get(configKeysPrefix.withSuffix("tenant_mode"_sr)); @@ -740,12 +739,10 @@ Future> createTenantTransaction(Transaction tr, TenantN Optional tenantEntry = wait(tenantEntryFuture); if (tenantEntry.present()) { - return Optional(); + return std::make_pair(tenantEntry.get(), false); } - state Optional lastIdVal = wait(safeThreadFutureToFuture(lastIdFuture)); Optional tenantDataPrefix = wait(safeThreadFutureToFuture(tenantDataPrefixFuture)); - if (tenantDataPrefix.present() && tenantDataPrefix.get().size() + TenantMapEntry::ROOT_PREFIX_SIZE > CLIENT_KNOBS->TENANT_PREFIX_SIZE_LIMIT) { TraceEvent(SevWarnAlways, "TenantPrefixTooLarge") @@ -757,8 +754,7 @@ Future> createTenantTransaction(Transaction tr, TenantN throw client_invalid_operation(); } - state TenantMapEntry newTenant(lastIdVal.present() ? TenantMapEntry::prefixToId(lastIdVal.get()) + 1 : 0, - tenantDataPrefix.present() ? (KeyRef)tenantDataPrefix.get() : ""_sr); + state TenantMapEntry newTenant(tenantId, tenantDataPrefix.present() ? (KeyRef)tenantDataPrefix.get() : ""_sr); state typename transaction_future_type::type prefixRangeFuture = tr->getRange(prefixRange(newTenant.prefix), 1); @@ -767,20 +763,20 @@ Future> createTenantTransaction(Transaction tr, TenantN throw tenant_prefix_allocator_conflict(); } - tr->set(tenantLastIdKey, TenantMapEntry::idToPrefix(newTenant.id)); tr->set(tenantMapKey, encodeTenantEntry(newTenant)); - return newTenant; + return std::make_pair(newTenant, true); } ACTOR template -Future createTenant(Reference db, TenantName name) { +Future createTenant(Reference db, TenantName name) { state Reference tr = db->createTransaction(); state bool firstTry = true; loop { try { tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); + state typename DB::TransactionT::template FutureT> lastIdFuture = tr->get(tenantLastIdKey); if (firstTry) { Optional entry = wait(tryGetTenantTransaction(tr, name)); @@ -791,7 +787,10 @@ Future createTenant(Reference db, TenantName name) { firstTry = false; } - state Optional newTenant = wait(createTenantTransaction(tr, name)); + Optional lastIdVal = wait(safeThreadFutureToFuture(lastIdFuture)); + int64_t tenantId = lastIdVal.present() ? TenantMapEntry::prefixToId(lastIdVal.get()) + 1 : 0; + tr->set(tenantLastIdKey, TenantMapEntry::idToPrefix(tenantId)); + state std::pair newTenant = wait(createTenantTransaction(tr, name, tenantId)); if (BUGGIFY) { throw commit_unknown_result(); @@ -805,11 +804,11 @@ Future createTenant(Reference db, TenantName name) { TraceEvent("CreatedTenant") .detail("Tenant", name) - .detail("TenantId", newTenant.present() ? newTenant.get().id : -1) - .detail("Prefix", newTenant.present() ? (StringRef)newTenant.get().prefix : "Unknown"_sr) + .detail("TenantId", newTenant.first.id) + .detail("Prefix", newTenant.first.prefix) .detail("Version", tr->getCommittedVersion()); - return Void(); + return newTenant.first; } catch (Error& e) { wait(safeThreadFutureToFuture(tr->onError(e))); } @@ -821,7 +820,6 @@ Future deleteTenantTransaction(Transaction tr, TenantNameRef name) { state Key tenantMapKey = name.withPrefix(tenantMapPrefix); tr->setOption(FDBTransactionOptions::RAW_ACCESS); - tr->setOption(FDBTransactionOptions::LOCK_AWARE); state Optional tenantEntry = wait(tryGetTenantTransaction(tr, name)); if (!tenantEntry.present()) { @@ -886,7 +884,6 @@ Future> listTenantsTransaction(Transaction state KeyRange range = KeyRangeRef(begin, end).withPrefix(tenantMapPrefix); tr->setOption(FDBTransactionOptions::RAW_ACCESS); - tr->setOption(FDBTransactionOptions::READ_LOCK_AWARE); state typename transaction_future_type::type listFuture = tr->getRange(firstGreaterOrEqual(range.begin), firstGreaterOrEqual(range.end), limit); diff --git a/fdbclient/SpecialKeySpace.actor.cpp b/fdbclient/SpecialKeySpace.actor.cpp index f2bedf7138..1ff19f442c 100644 --- a/fdbclient/SpecialKeySpace.actor.cpp +++ b/fdbclient/SpecialKeySpace.actor.cpp @@ -2772,7 +2772,24 @@ Future TenantMapRangeImpl::getRange(ReadYourWritesTransaction* ryw, return getTenantList(ryw, kr, limitsHint); } -ACTOR Future deleteTenantRange(ReadYourWritesTransaction* ryw, TenantName beginTenant, TenantName endTenant) { +ACTOR Future createTenants(ReadYourWritesTransaction* ryw, std::vector tenants) { + Optional lastIdVal = wait(ryw->getTransaction().get(tenantLastIdKey)); + int64_t previousId = lastIdVal.present() ? TenantMapEntry::prefixToId(lastIdVal.get()) : -1; + + std::vector> createFutures; + for (auto tenant : tenants) { + createFutures.push_back( + success(ManagementAPI::createTenantTransaction(&ryw->getTransaction(), tenant, ++previousId))); + } + + ryw->getTransaction().set(tenantLastIdKey, TenantMapEntry::idToPrefix(previousId)); + wait(waitForAll(createFutures)); + return Void(); +} + +ACTOR Future deleteTenantRange(ReadYourWritesTransaction* ryw, + TenantNameRef beginTenant, + TenantNameRef endTenant) { std::map tenants = wait( ManagementAPI::listTenantsTransaction(&ryw->getTransaction(), beginTenant, endTenant, CLIENT_KNOBS->TOO_MANY)); @@ -2795,6 +2812,7 @@ ACTOR Future deleteTenantRange(ReadYourWritesTransaction* ryw, TenantName Future> TenantMapRangeImpl::commit(ReadYourWritesTransaction* ryw) { auto ranges = ryw->getSpecialKeySpaceWriteMap().containedRanges(range); + std::vector tenantsToCreate; std::vector> tenantManagementFutures; for (auto range : ranges) { if (!range.value().first) { @@ -2807,8 +2825,7 @@ Future> TenantMapRangeImpl::commit(ReadYourWritesTransacti .removePrefix(TenantMapRangeImpl::submoduleRange.begin); if (range.value().second.present()) { - tenantManagementFutures.push_back( - success(ManagementAPI::createTenantTransaction(&ryw->getTransaction(), tenantName))); + tenantsToCreate.push_back(tenantName); } else { // For a single key clear, just issue the delete if (KeyRangeRef(range.begin(), range.end()).singleKeyRange()) { @@ -2827,5 +2844,9 @@ Future> TenantMapRangeImpl::commit(ReadYourWritesTransacti } } + if (tenantsToCreate.size()) { + tenantManagementFutures.push_back(createTenants(ryw, tenantsToCreate)); + } + return tag(waitForAll(tenantManagementFutures), Optional()); } diff --git a/fdbserver/tester.actor.cpp b/fdbserver/tester.actor.cpp index 4d890131b4..8df0a2c0ba 100644 --- a/fdbserver/tester.actor.cpp +++ b/fdbserver/tester.actor.cpp @@ -1629,7 +1629,7 @@ ACTOR Future runTests(Reference> tenantFutures; for (auto tenant : tenantsToCreate) { TraceEvent("CreatingTenant").detail("Tenant", tenant); - tenantFutures.push_back(ManagementAPI::createTenant(cx.getReference(), tenant)); + tenantFutures.push_back(success(ManagementAPI::createTenant(cx.getReference(), tenant))); } wait(waitForAll(tenantFutures)); diff --git a/fdbserver/workloads/BlobGranuleCorrectnessWorkload.actor.cpp b/fdbserver/workloads/BlobGranuleCorrectnessWorkload.actor.cpp index d07f948277..d397b14ced 100644 --- a/fdbserver/workloads/BlobGranuleCorrectnessWorkload.actor.cpp +++ b/fdbserver/workloads/BlobGranuleCorrectnessWorkload.actor.cpp @@ -206,30 +206,14 @@ struct BlobGranuleCorrectnessWorkload : TestWorkload { if (BGW_DEBUG) { fmt::print("Setting up blob granule range for tenant {0}\n", name.printable()); } - state Reference tr = makeReference(cx); - loop { - try { - tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); - tr->setOption(FDBTransactionOptions::PRIORITY_SYSTEM_IMMEDIATE); - state Optional entry = wait(ManagementAPI::createTenantTransaction(tr, name)); - if (!entry.present()) { - // if tenant already exists because of retry, load it - wait(store(entry, ManagementAPI::tryGetTenantTransaction(tr, name))); - ASSERT(entry.present()); - } + TenantMapEntry entry = wait(ManagementAPI::createTenant(cx.getReference(), name)); - wait(tr->commit()); - if (BGW_DEBUG) { - fmt::print("Set up blob granule range for tenant {0}: {1}\n", - name.printable(), - entry.get().prefix.printable()); - } - return entry.get(); - } catch (Error& e) { - wait(tr->onError(e)); - } + if (BGW_DEBUG) { + fmt::print("Set up blob granule range for tenant {0}: {1}\n", name.printable(), entry.prefix.printable()); } + + return entry; } std::string description() const override { return "BlobGranuleCorrectnessWorkload"; } diff --git a/fdbserver/workloads/FuzzApiCorrectness.actor.cpp b/fdbserver/workloads/FuzzApiCorrectness.actor.cpp index bec0240257..096a20d551 100644 --- a/fdbserver/workloads/FuzzApiCorrectness.actor.cpp +++ b/fdbserver/workloads/FuzzApiCorrectness.actor.cpp @@ -225,7 +225,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { // The last tenant will not be created if (i < self->numTenants) { - tenantFutures.push_back(ManagementAPI::createTenant(cx.getReference(), tenantName)); + tenantFutures.push_back(::success(ManagementAPI::createTenant(cx.getReference(), tenantName))); self->createdTenants.insert(tenantName); } } diff --git a/fdbserver/workloads/TenantManagement.actor.cpp b/fdbserver/workloads/TenantManagement.actor.cpp index 75d6da06c7..f9b7203d59 100644 --- a/fdbserver/workloads/TenantManagement.actor.cpp +++ b/fdbserver/workloads/TenantManagement.actor.cpp @@ -58,7 +58,7 @@ struct TenantManagementWorkload : TestWorkload { enum class OperationType { SPECIAL_KEYS, MANAGEMENT_DATABASE, MANAGEMENT_TRANSACTION }; static OperationType randomOperationType() { - int randomNum = deterministicRandom()->randomInt(1, 3); + int randomNum = deterministicRandom()->randomInt(0, 3); if (randomNum == 0) { return OperationType::SPECIAL_KEYS; } else if (randomNum == 1) { @@ -163,12 +163,20 @@ struct TenantManagementWorkload : TestWorkload { wait(tr->commit()); } else if (operationType == OperationType::MANAGEMENT_DATABASE) { ASSERT(tenantsToCreate.size() == 1); - wait(ManagementAPI::createTenant(cx.getReference(), *tenantsToCreate.begin())); + wait(success(ManagementAPI::createTenant(cx.getReference(), *tenantsToCreate.begin()))); } else { tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); + + Optional lastIdVal = wait(tr->get(tenantLastIdKey)); + int64_t previousId = lastIdVal.present() ? TenantMapEntry::prefixToId(lastIdVal.get()) : -1; + + std::vector> createFutures; for (auto tenant : tenantsToCreate) { - Optional _ = wait(ManagementAPI::createTenantTransaction(tr, tenant)); + createFutures.push_back( + success(ManagementAPI::createTenantTransaction(tr, tenant, ++previousId))); } + tr->set(tenantLastIdKey, TenantMapEntry::idToPrefix(previousId)); + wait(waitForAll(createFutures)); wait(tr->commit()); } From 49a9a850d6bcc221da762048e9efd2c255c49259 Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Fri, 3 Jun 2022 16:41:32 -0700 Subject: [PATCH 3/3] Move the lock aware option into the database version of the tenant functions --- fdbclient/GenericManagementAPI.actor.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fdbclient/GenericManagementAPI.actor.h b/fdbclient/GenericManagementAPI.actor.h index fd9ca36101..3268cd64d5 100644 --- a/fdbclient/GenericManagementAPI.actor.h +++ b/fdbclient/GenericManagementAPI.actor.h @@ -684,6 +684,7 @@ Future> tryGetTenant(Reference db, TenantName name) loop { try { tr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS); + tr->setOption(FDBTransactionOptions::READ_LOCK_AWARE); Optional entry = wait(tryGetTenantTransaction(tr, name)); return entry; } catch (Error& e) { @@ -776,6 +777,7 @@ Future createTenant(Reference db, TenantName name) { loop { try { tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); + tr->setOption(FDBTransactionOptions::LOCK_AWARE); state typename DB::TransactionT::template FutureT> lastIdFuture = tr->get(tenantLastIdKey); if (firstTry) { @@ -846,6 +848,7 @@ Future deleteTenant(Reference db, TenantName name) { loop { try { tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); + tr->setOption(FDBTransactionOptions::LOCK_AWARE); if (firstTry) { Optional entry = wait(tryGetTenantTransaction(tr, name)); @@ -907,6 +910,7 @@ Future> listTenants(Reference db, loop { try { tr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS); + tr->setOption(FDBTransactionOptions::READ_LOCK_AWARE); std::map tenants = wait(listTenantsTransaction(tr, begin, end, limit)); return tenants; } catch (Error& e) {