From 592f31755e97dd896296548bd3e21322958ac992 Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Fri, 25 Feb 2022 10:54:43 -0800 Subject: [PATCH] Fixes to the new tenant tests --- fdbserver/tester.actor.cpp | 1 + .../workloads/FuzzApiCorrectness.actor.cpp | 71 ++++++++++--------- .../workloads/TenantManagement.actor.cpp | 60 ++++++++++------ 3 files changed, 78 insertions(+), 54 deletions(-) diff --git a/fdbserver/tester.actor.cpp b/fdbserver/tester.actor.cpp index dff58e03eb..911eb14405 100644 --- a/fdbserver/tester.actor.cpp +++ b/fdbserver/tester.actor.cpp @@ -1502,6 +1502,7 @@ ACTOR Future runTests(Reference tr) const { // We should always ignore these. if (e.code() == error_code_used_during_commit || e.code() == error_code_transaction_too_old || e.code() == error_code_future_version || e.code() == error_code_transaction_cancelled || @@ -71,6 +71,7 @@ struct ExceptionContract { evt.error(e) .detail("Thrown", true) .detail("Expected", i->second == Possible ? "possible" : "always") + .detail("Tenant", tr->getTenant()) .backtrace(); if (augment) augment(evt); @@ -78,20 +79,21 @@ struct ExceptionContract { } TraceEvent evt(SevError, func.c_str()); - evt.error(e).detail("Thrown", true).detail("Expected", "never").backtrace(); + evt.error(e).detail("Thrown", true).detail("Expected", "never").detail("Tenant", tr->getTenant()).backtrace(); if (augment) augment(evt); throw e; } // Return true if we should have thrown, but didn't. - void handleNotThrown() const { + void handleNotThrown(Reference tr) const { for (auto i : expected) { if (i.second == Always) { TraceEvent evt(SevError, func.c_str()); evt.error(Error::fromUnvalidatedCode(i.first)) .detail("Thrown", false) .detail("Expected", "always") + .detail("Tenant", tr->getTenant()) .backtrace(); if (augment) augment(evt); @@ -172,6 +174,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { conflictRange = KeyRangeRef(LiteralStringRef("\xfe"), LiteralStringRef("\xfe\x00")); TraceEvent("FuzzApiCorrectnessConfiguration") .detail("Nodes", nodes) + .detail("NumTenants", numTenants) .detail("InitialKeyDensity", initialKeyDensity) .detail("AdjacentKeys", adjacentKeys) .detail("ValueSizeMin", valueSizeRange.first) @@ -198,7 +201,9 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { std::string description() const override { return "FuzzApiCorrectness"; } static TenantName getTenant(int num) { return TenantNameRef(format("tenant_%d", num)); } - bool hasTenant(Optional tenant) { return !tenant.present() || createdTenants.count(tenant.get()); } + bool canUseTenant(Optional tenant) { + return !tenant.present() || createdTenants.count(tenant.get()) || useSystemKeys; + } Future setup(Database const& cx) override { if (clientId == 0) { @@ -259,7 +264,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { ACTOR Future loadAndRun(FuzzApiCorrectnessWorkload* self) { state double startTime = now(); - state int nodesPerTenant = self->nodes / (self->numTenants + 1); + state int nodesPerTenant = std::max(1, self->nodes / (self->numTenants + 1)); state int keysPerBatch = std::min(1000, 1 + CLIENT_KNOBS->TRANSACTION_SIZE_LIMIT / 2 / @@ -270,8 +275,9 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { for (; tenantNum < self->numTenants; ++tenantNum) { state int i = 0; for (; i < nodesPerTenant; i += keysPerBatch) { - state Reference tr = - tenantNum < 0 ? self->db->createTransaction() : self->tenants[i]->createTransaction(); + state Reference tr = tenantNum < 0 + ? self->db->createTransaction() + : self->tenants[tenantNum]->createTransaction(); loop { if (now() - startTime > self->testDuration) return Void(); @@ -343,7 +349,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { state std::vector> operations; state int waitLocation = 0; - int tenantNum = deterministicRandom()->randomInt(-1, self->tenants.size()); + state int tenantNum = deterministicRandom()->randomInt(-1, self->tenants.size()); if (tenantNum == -1) { tr = self->db->createTransaction(); } else { @@ -436,10 +442,10 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { try { value_type result = wait(timeoutError(BaseTest::runTest2(tr, &self), 1000)); - self.contract.handleNotThrown(); + self.contract.handleNotThrown(tr); return self.errorCheck(tr, result); } catch (Error& e) { - self.contract.handleException(e); + self.contract.handleException(e, tr); } return Void(); } @@ -456,7 +462,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { value_type result = wait(future); if (future.isError()) { - self->contract.handleException(future.getError()); + self->contract.handleException(future.getError(), tr); } else { ASSERT(future.isValid()); } @@ -696,7 +702,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { LiteralStringRef("auto_coordinators") .withPrefix(SpecialKeySpace::getModuleRange(SpecialKeySpace::MODULE::MANAGEMENT).begin))), std::make_pair(error_code_tenant_not_found, - ExceptionContract::requiredIf(!workload->hasTenant(tr->getTenant()))) + ExceptionContract::possibleIf(!workload->canUseTenant(tr->getTenant()))) }; } @@ -706,7 +712,8 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { void augmentTrace(TraceEvent& e) const override { base_type::augmentTrace(e); - e.detail("Key", printable(key)); + e.detail("Key", key); + e.detail("Size", key.size()); } }; @@ -724,7 +731,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { std::make_pair(error_code_client_invalid_operation, ExceptionContract::Possible), std::make_pair(error_code_accessed_unreadable, ExceptionContract::Possible), std::make_pair(error_code_tenant_not_found, - ExceptionContract::requiredIf(!workload->hasTenant(tr->getTenant()))) }; + ExceptionContract::possibleIf(!workload->canUseTenant(tr->getTenant()))) }; } ThreadFuture createFuture(Reference tr) override { @@ -776,7 +783,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { std::make_pair(error_code_special_keys_api_failure, ExceptionContract::possibleIf(isSpecialKeyRange)), std::make_pair(error_code_accessed_unreadable, ExceptionContract::Possible), std::make_pair(error_code_tenant_not_found, - ExceptionContract::requiredIf(!workload->hasTenant(tr->getTenant()))) + ExceptionContract::possibleIf(!workload->canUseTenant(tr->getTenant()))) }; } @@ -822,7 +829,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { std::make_pair(error_code_special_keys_api_failure, ExceptionContract::possibleIf(isSpecialKeyRange)), std::make_pair(error_code_accessed_unreadable, ExceptionContract::Possible), std::make_pair(error_code_tenant_not_found, - ExceptionContract::requiredIf(!workload->hasTenant(tr->getTenant()))) + ExceptionContract::possibleIf(!workload->canUseTenant(tr->getTenant()))) }; } @@ -888,7 +895,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { autoCoordinatorSpecialKey < key2)), std::make_pair(error_code_accessed_unreadable, ExceptionContract::Possible), std::make_pair(error_code_tenant_not_found, - ExceptionContract::requiredIf(!workload->hasTenant(tr->getTenant()))) + ExceptionContract::possibleIf(!workload->canUseTenant(tr->getTenant()))) }; } @@ -899,7 +906,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { void augmentTrace(TraceEvent& e) const override { base_type::augmentTrace(e); - e.detail("Key1", printable(key1)).detail("Key2", printable(key2)).detail("Limit", limit); + e.detail("Key1", key1).detail("Key2", key2).detail("Limit", limit); } }; @@ -942,7 +949,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { (autoCoordinatorSpecialKey < key2))), std::make_pair(error_code_accessed_unreadable, ExceptionContract::Possible), std::make_pair(error_code_tenant_not_found, - ExceptionContract::requiredIf(!workload->hasTenant(tr->getTenant()))) + ExceptionContract::possibleIf(!workload->canUseTenant(tr->getTenant()))) }; } @@ -953,7 +960,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { void augmentTrace(TraceEvent& e) const override { base_type::augmentTrace(e); - e.detail("Key1", printable(key1)).detail("Key2", printable(key2)); + e.detail("Key1", key1).detail("Key2", key2); std::stringstream ss; ss << "(" << limits.rows << ", " << limits.minRows << ", " << limits.bytes << ")"; e.detail("Limits", ss.str()); @@ -969,7 +976,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { key = makeKey(); contract = { std::make_pair(error_code_client_invalid_operation, ExceptionContract::Possible), std::make_pair(error_code_tenant_not_found, - ExceptionContract::requiredIf(!workload->hasTenant(tr->getTenant()))) }; + ExceptionContract::requiredIf(!workload->canUseTenant(tr->getTenant()))) }; } ThreadFuture createFuture(Reference tr) override { @@ -978,7 +985,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { void augmentTrace(TraceEvent& e) const override { base_type::augmentTrace(e); - e.detail("Key", printable(key)); + e.detail("Key", key); } }; @@ -1001,7 +1008,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { void augmentTrace(TraceEvent& e) const override { base_type::augmentTrace(e); - e.detail("Key1", printable(key1)).detail("Key2", printable(key2)); + e.detail("Key1", key1).detail("Key2", key2); } }; @@ -1076,7 +1083,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { void augmentTrace(TraceEvent& e) const override { base_type::augmentTrace(e); - e.detail("Key", printable(key)).detail("Value", printable(value)).detail("Op", op).detail("Pos", pos); + e.detail("Key", key).detail("Value", value).detail("Op", op).detail("Pos", pos); } }; @@ -1115,7 +1122,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { void augmentTrace(TraceEvent& e) const override { base_type::augmentTrace(e); - e.detail("Key", printable(key)).detail("Value", printable(value)); + e.detail("Key", key).detail("Value", value); } }; @@ -1154,7 +1161,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { void augmentTrace(TraceEvent& e) const override { base_type::augmentTrace(e); - e.detail("Key1", printable(key1)).detail("Key2", printable(key2)); + e.detail("Key1", key1).detail("Key2", key2); } }; @@ -1193,7 +1200,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { void augmentTrace(TraceEvent& e) const override { base_type::augmentTrace(e); - e.detail("Key1", printable(key1)).detail("Key2", printable(key2)); + e.detail("Key1", key1).detail("Key2", key2); } }; @@ -1222,7 +1229,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { void augmentTrace(TraceEvent& e) const override { base_type::augmentTrace(e); - e.detail("Key", printable(key)); + e.detail("Key", key); } }; @@ -1246,14 +1253,14 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { std::make_pair(error_code_timed_out, ExceptionContract::Possible), std::make_pair(error_code_accessed_unreadable, ExceptionContract::Possible), std::make_pair(error_code_tenant_not_found, - ExceptionContract::requiredIf(!workload->hasTenant(tr->getTenant()))) }; + ExceptionContract::possibleIf(!workload->canUseTenant(tr->getTenant()))) }; } ThreadFuture createFuture(Reference tr) override { return tr->watch(key); } void augmentTrace(TraceEvent& e) const override { base_type::augmentTrace(e); - e.detail("Key", printable(key)); + e.detail("Key", key); } }; @@ -1276,7 +1283,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { void augmentTrace(TraceEvent& e) const override { base_type::augmentTrace(e); - e.detail("Key1", printable(key1)).detail("Key2", printable(key2)); + e.detail("Key1", key1).detail("Key2", key2); } }; @@ -1361,7 +1368,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { void augmentTrace(TraceEvent& e) const override { base_type::augmentTrace(e); - e.detail("Op", op).detail("Val", printable(val)); + e.detail("Op", op).detail("Val", val); } }; diff --git a/fdbserver/workloads/TenantManagement.actor.cpp b/fdbserver/workloads/TenantManagement.actor.cpp index 59e78dea85..d9d0b3e865 100644 --- a/fdbserver/workloads/TenantManagement.actor.cpp +++ b/fdbserver/workloads/TenantManagement.actor.cpp @@ -46,6 +46,8 @@ struct TenantManagementWorkload : TestWorkload { const Key keyName = "key"_sr; const Key tenantSubspaceKey = "tenant_subspace"_sr; const Value noTenantValue = "no_tenant"_sr; + const TenantName tenantNamePrefix = "tenant_management_workload_"_sr; + TenantName localTenantNamePrefix; int maxTenants; double testDuration; @@ -53,6 +55,8 @@ struct TenantManagementWorkload : TestWorkload { TenantManagementWorkload(WorkloadContext const& wcx) : TestWorkload(wcx) { maxTenants = getOption(options, "maxTenants"_sr, 1000); testDuration = getOption(options, "testDuration"_sr, 60.0); + + localTenantNamePrefix = format("%stenant_%d_", tenantNamePrefix.toString().c_str(), clientId); } std::string description() const override { return "TenantManagement"; } @@ -68,6 +72,7 @@ struct TenantManagementWorkload : TestWorkload { tr.setOption(FDBTransactionOptions::RAW_ACCESS); tr.set(self->keyName, self->noTenantValue); tr.set(self->tenantSubspaceKey, self->tenantSubspace); + tr.set(tenantDataPrefixKey, self->tenantSubspace); wait(tr.commit()); break; } catch (Error& e) { @@ -94,9 +99,10 @@ struct TenantManagementWorkload : TestWorkload { return Void(); } - TenantName chooseTenantName() { - TenantName tenant(format("tenant_%d_%d", clientId, deterministicRandom()->randomInt(0, maxTenants))); - if (deterministicRandom()->random01() < 0.02) { + TenantName chooseTenantName(bool allowSystemTenant) { + TenantName tenant( + format("%s%d", localTenantNamePrefix.toString().c_str(), deterministicRandom()->randomInt(0, maxTenants))); + if (allowSystemTenant && deterministicRandom()->random01() < 0.02) { tenant = tenant.withPrefix("\xff"_sr); } @@ -104,7 +110,7 @@ struct TenantManagementWorkload : TestWorkload { } ACTOR Future createTenant(Database cx, TenantManagementWorkload* self) { - state TenantName tenant = self->chooseTenantName(); + state TenantName tenant = self->chooseTenantName(true); state bool alreadyExists = self->createdTenants.count(tenant); try { wait(ManagementAPI::createTenant(cx.getReference(), tenant)); @@ -117,6 +123,7 @@ struct TenantManagementWorkload : TestWorkload { ASSERT(entry.get().prefix.startsWith(self->tenantSubspace)); self->maxId = entry.get().id; + self->createdTenants[tenant] = TenantState(entry.get().id, true); state bool insertData = deterministicRandom()->random01() < 0.5; if (insertData) { @@ -131,6 +138,8 @@ struct TenantManagementWorkload : TestWorkload { } } + self->createdTenants[tenant].empty = false; + tr = Transaction(cx); loop { try { @@ -145,14 +154,14 @@ struct TenantManagementWorkload : TestWorkload { } } - self->createdTenants[tenant] = TenantState(entry.get().id, insertData); + wait(self->checkTenant(cx, self, tenant, self->createdTenants[tenant])); } catch (Error& e) { if (e.code() == error_code_tenant_already_exists) { ASSERT(alreadyExists); } else if (e.code() == error_code_invalid_tenant_name) { ASSERT(tenant.startsWith("\xff"_sr)); } else { - TraceEvent(SevError, "CreateTenantFailure").detail("TenantName", tenant).error(e); + TraceEvent(SevError, "CreateTenantFailure").error(e).detail("TenantName", tenant); } } @@ -160,7 +169,7 @@ struct TenantManagementWorkload : TestWorkload { } ACTOR Future deleteTenant(Database cx, TenantManagementWorkload* self) { - state TenantName tenant = self->chooseTenantName(); + state TenantName tenant = self->chooseTenantName(true); auto itr = self->createdTenants.find(tenant); state bool alreadyExists = itr != self->createdTenants.end(); @@ -194,7 +203,7 @@ struct TenantManagementWorkload : TestWorkload { } else if (e.code() == error_code_tenant_not_empty) { ASSERT(!isEmpty); } else { - TraceEvent(SevError, "DeleteTenantFailure").detail("TenantName", tenant).error(e); + TraceEvent(SevError, "DeleteTenantFailure").error(e).detail("TenantName", tenant); } } @@ -208,7 +217,7 @@ struct TenantManagementWorkload : TestWorkload { state Transaction tr(cx, tenant); loop { try { - RangeResult result = wait(tr.getRange(KeyRangeRef(""_sr, "\xff"_sr), 2)); + state RangeResult result = wait(tr.getRange(KeyRangeRef(""_sr, "\xff"_sr), 2)); if (tenantState.empty) { ASSERT(result.size() == 0); } else { @@ -226,7 +235,7 @@ struct TenantManagementWorkload : TestWorkload { } ACTOR Future getTenant(Database cx, TenantManagementWorkload* self) { - state TenantName tenant = self->chooseTenantName(); + state TenantName tenant = self->chooseTenantName(true); auto itr = self->createdTenants.find(tenant); state bool alreadyExists = itr != self->createdTenants.end(); state TenantState tenantState = itr->second; @@ -240,7 +249,7 @@ struct TenantManagementWorkload : TestWorkload { if (e.code() == error_code_tenant_not_found) { ASSERT(!alreadyExists); } else { - TraceEvent(SevError, "GetTenantFailure").detail("TenantName", tenant).error(e); + TraceEvent(SevError, "GetTenantFailure").error(e).detail("TenantName", tenant); } } @@ -248,10 +257,14 @@ struct TenantManagementWorkload : TestWorkload { } ACTOR Future listTenants(Database cx, TenantManagementWorkload* self) { - state TenantName beginTenant = self->chooseTenantName(); - state TenantName endTenant = self->chooseTenantName(); + state TenantName beginTenant = self->chooseTenantName(false); + state TenantName endTenant = self->chooseTenantName(false); state int limit = std::min(CLIENT_KNOBS->TOO_MANY, deterministicRandom()->randomInt(0, self->maxTenants * 2)); + if (beginTenant > endTenant) { + std::swap(beginTenant, endTenant); + } + try { Standalone> tenants = wait(ManagementAPI::listTenants(cx.getReference(), beginTenant, endTenant, limit)); @@ -259,21 +272,23 @@ struct TenantManagementWorkload : TestWorkload { ASSERT(tenants.size() <= limit); int index = 0; - auto itr = self->createdTenants.begin(); + auto itr = self->createdTenants.lower_bound(beginTenant); for (; index < tenants.size(); ++itr) { ASSERT(itr != self->createdTenants.end()); ASSERT(itr->first == tenants[index++]); } - ASSERT(tenants.size() == limit || itr == self->createdTenants.end()); - if (tenants.size() == limit) { - ASSERT(itr == self->createdTenants.end() || itr->first >= endTenant); + if (!(tenants.size() == limit || itr == self->createdTenants.end())) { + for (auto tenant : self->createdTenants) { + TraceEvent("ExistingTenant").detail("Tenant", tenant.first); + } } + ASSERT(tenants.size() == limit || itr == self->createdTenants.end() || itr->first >= endTenant); } catch (Error& e) { TraceEvent(SevError, "ListTenantFailure") + .error(e) .detail("BeginTenant", beginTenant) - .detail("EndTenant", endTenant) - .error(e); + .detail("EndTenant", endTenant); } return Void(); @@ -315,14 +330,15 @@ struct TenantManagementWorkload : TestWorkload { state std::map::iterator itr = self->createdTenants.begin(); state std::vector> checkTenants; - state TenantName beginTenant = ""_sr; + state TenantName beginTenant = ""_sr.withPrefix(self->localTenantNamePrefix); + state TenantName endTenant = "\xff\xff"_sr.withPrefix(self->localTenantNamePrefix); loop { Standalone> tenants = - wait(ManagementAPI::listTenants(cx.getReference(), beginTenant, "\xff\xff"_sr, 1000)); + wait(ManagementAPI::listTenants(cx.getReference(), beginTenant, endTenant, 1000)); for (auto tenant : tenants) { - ASSERT(!tenant.startsWith("\xff"_sr)); + ASSERT(itr != self->createdTenants.end()); ASSERT(tenant == itr->first); checkTenants.push_back(self->checkTenant(cx, self, tenant, itr->second)); ++itr;