From 13fac35f5331af94232e442b81231e37541020f4 Mon Sep 17 00:00:00 2001 From: Yanqin Jin Date: Wed, 5 Jul 2023 06:53:08 -0700 Subject: [PATCH 1/2] SNOW-791059 Verify no data outside tenants in REQUIRED mode (#489) If tenant mode is REQUIRED, then we should verify that in the normal key space, no data exists outside tenants' prefixes. This applies to data clusters (also known as partition clusters) in a metacluster and standalone clusters with tenants. For the management cluster of a metacluster, we should verify that no data exists outside the prefix ranges specified by `tenant/` and `metacluster/` in the normal key space. Test plan: devRunCorrectnessFiltered +Metacluster* +Tenant* --max-runs 100000 20230702-052847-yajin-082705d269588494. 0 Failure devRunCorrectness --max-runs 100000 20230702-134219-yajin-e9cce7bd165e70a9. 1 Failure, unrelated to this change --- .../fdbclient/TenantManagement.actor.h | 11 +++ .../metacluster/TenantConsistency.actor.h | 76 +++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/fdbclient/include/fdbclient/TenantManagement.actor.h b/fdbclient/include/fdbclient/TenantManagement.actor.h index b1062f6927..cb2a8bf94a 100644 --- a/fdbclient/include/fdbclient/TenantManagement.actor.h +++ b/fdbclient/include/fdbclient/TenantManagement.actor.h @@ -133,6 +133,17 @@ int64_t computeNextTenantId(int64_t tenantId, int64_t delta); int64_t getMaxAllowableTenantId(int64_t curTenantId); int64_t getTenantIdPrefix(int64_t tenantId); +ACTOR template +Future getEffectiveTenantMode(Transaction tr) { + state typename transaction_future_type::type tenantModeFuture = + tr->get(configKeysPrefix.withSuffix("tenant_mode"_sr)); + state ClusterType clusterType; + state ValueReadResult tenantModeValue; + wait(store(clusterType, getClusterType(tr)) && store(tenantModeValue, safeThreadFutureToFuture(tenantModeFuture))); + TenantMode tenantMode = TenantMode::fromValue(tenantModeValue.castTo()); + return tenantModeForClusterType(clusterType, tenantMode); +} + // Returns true if the specified ID has already been deleted and false if not. If the ID is old enough // that we no longer keep tombstones for it, an error is thrown. ACTOR template diff --git a/metacluster/include/metacluster/TenantConsistency.actor.h b/metacluster/include/metacluster/TenantConsistency.actor.h index b1f3d9c9fc..08a83fce3d 100644 --- a/metacluster/include/metacluster/TenantConsistency.actor.h +++ b/metacluster/include/metacluster/TenantConsistency.actor.h @@ -138,10 +138,86 @@ private: } } + ACTOR static Future validateNoDataInRanges(Reference db, Standalone> ranges) { + state std::vector> rangeReadFutures; + for (const auto& range : ranges) { + Future f = runTransaction(db, [range](Reference tr) { + tr->setOption(FDBTransactionOptions::RAW_ACCESS); + return safeThreadFutureToFuture(tr->getRange(range, 1)); + }); + rangeReadFutures.emplace_back(f); + } + wait(waitForAll(rangeReadFutures)); + for (size_t i = 0; i < ranges.size(); ++i) { + auto f = rangeReadFutures[i]; + ASSERT(f.isReady()); + RangeReadResult rangeReadResult = f.get(); + if (!rangeReadResult.empty()) { + TraceEvent(SevError, "DataOutsideTenants") + .detail("Count", rangeReadFutures.size()) + .detail("Index", i) + .detail("Begin", ranges[i].begin.toHexString()) + .detail("End", ranges[i].end.toHexString()) + .detail("RangeReadResult", rangeReadResult.toString()) + .detail("ReadThrough", rangeReadResult.getReadThrough().toHexString()); + ASSERT(false); + } + } + return Void(); + } + + ACTOR static Future checkNoDataOutsideTenantsInRequiredMode( + TenantConsistencyCheck* self) { + state Future tenantModeFuture = + runTransaction(self->tenantData.db, [](Reference tr) { + tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); + return TenantAPI::getEffectiveTenantMode(tr); + }); + + TenantMode tenantMode = wait(tenantModeFuture); + if (tenantMode != TenantMode::REQUIRED) { + return Void(); + } + CODE_PROBE(true, "Data or standalone cluster with tenant_mode=required"); + + int64_t prevId = -1; + Key prevPrefix; + Key prevGapStart; + Standalone> gaps; + for (const auto& [id, entry] : self->tenantData.tenantMap) { + ASSERT(id > prevId); + ASSERT_EQ(TenantAPI::idToPrefix(id), entry.prefix); + if (prevId >= 0) { + ASSERT_GT(entry.prefix, prevPrefix); + } + ASSERT_GE(entry.prefix, prevGapStart); + gaps.push_back_deep(gaps.arena(), KeyRangeRef(prevGapStart, entry.prefix)); + prevGapStart = strinc(entry.prefix); + prevId = id; + prevPrefix = entry.prefix; + } + ASSERT_LE(prevGapStart, "\xff"_sr); + gaps.push_back_deep(gaps.arena(), KeyRangeRef(prevGapStart, "\xff"_sr)); + wait(validateNoDataInRanges(self->tenantData.db, gaps)); + return Void(); + } + + ACTOR static Future checkNoDataOutsideTenantsInRequiredMode( + TenantConsistencyCheck* self) { + // Check that no data exists outside of the metacluster metadata subspace + state Standalone> gaps; + gaps.push_back_deep(gaps.arena(), KeyRangeRef(""_sr, "metacluster/"_sr)); + gaps.push_back_deep(gaps.arena(), KeyRangeRef("metacluster0"_sr, "tenant/"_sr)); + gaps.push_back_deep(gaps.arena(), KeyRangeRef("tenant0"_sr, "\xff"_sr)); + wait(validateNoDataInRanges(self->tenantData.db, gaps)); + return Void(); + } + ACTOR static Future run(TenantConsistencyCheck* self) { wait(self->tenantData.load()); self->validateTenantMetadata(self->tenantData); self->checkTenantTombstones(); + wait(checkNoDataOutsideTenantsInRequiredMode(self)); return Void(); } From 2c8b6823109bead4469da6610879e71a25234d04 Mon Sep 17 00:00:00 2001 From: Yanqin Jin Date: Wed, 5 Jul 2023 15:49:29 -0700 Subject: [PATCH 2/2] Fix build issues --- .../include/fdbclient/TenantManagement.actor.h | 6 +++--- .../metacluster/TenantConsistency.actor.h | 18 +++++++++++++----- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/fdbclient/include/fdbclient/TenantManagement.actor.h b/fdbclient/include/fdbclient/TenantManagement.actor.h index cb2a8bf94a..64b52a231b 100644 --- a/fdbclient/include/fdbclient/TenantManagement.actor.h +++ b/fdbclient/include/fdbclient/TenantManagement.actor.h @@ -135,10 +135,10 @@ int64_t getTenantIdPrefix(int64_t tenantId); ACTOR template Future getEffectiveTenantMode(Transaction tr) { - state typename transaction_future_type::type tenantModeFuture = + state typename transaction_future_type>::type tenantModeFuture = tr->get(configKeysPrefix.withSuffix("tenant_mode"_sr)); state ClusterType clusterType; - state ValueReadResult tenantModeValue; + state Optional tenantModeValue; wait(store(clusterType, getClusterType(tr)) && store(tenantModeValue, safeThreadFutureToFuture(tenantModeFuture))); TenantMode tenantMode = TenantMode::fromValue(tenantModeValue.castTo()); return tenantModeForClusterType(clusterType, tenantMode); @@ -841,4 +841,4 @@ Future>> listTenantGrou } // namespace TenantAPI #include "flow/unactorcompiler.h" -#endif \ No newline at end of file +#endif diff --git a/metacluster/include/metacluster/TenantConsistency.actor.h b/metacluster/include/metacluster/TenantConsistency.actor.h index 08a83fce3d..e6474c1fe9 100644 --- a/metacluster/include/metacluster/TenantConsistency.actor.h +++ b/metacluster/include/metacluster/TenantConsistency.actor.h @@ -31,6 +31,7 @@ #include "fdbclient/FDBOptions.g.h" #include "fdbclient/KeyBackedTypes.actor.h" #include "flow/BooleanParam.h" +#include "flow/ThreadHelper.actor.h" #include "fdbclient/Tenant.h" #include "fdbclient/TenantData.actor.h" #include "fdbclient/TenantManagement.actor.h" @@ -139,19 +140,26 @@ private: } ACTOR static Future validateNoDataInRanges(Reference db, Standalone> ranges) { - state std::vector> rangeReadFutures; + state std::vector> rangeReadFutures; + state std::vector::type> + rangeReadThreadFutures; + std::vector::type>* ptr = + std::addressof(rangeReadThreadFutures); for (const auto& range : ranges) { - Future f = runTransaction(db, [range](Reference tr) { + Future future = runTransaction(db, [range, ptr](Reference tr) { tr->setOption(FDBTransactionOptions::RAW_ACCESS); - return safeThreadFutureToFuture(tr->getRange(range, 1)); + typename transaction_future_type::type f = + tr->getRange(range, 1); + ptr->emplace_back(f); + return safeThreadFutureToFuture(f); }); - rangeReadFutures.emplace_back(f); + rangeReadFutures.emplace_back(future); } wait(waitForAll(rangeReadFutures)); for (size_t i = 0; i < ranges.size(); ++i) { auto f = rangeReadFutures[i]; ASSERT(f.isReady()); - RangeReadResult rangeReadResult = f.get(); + RangeResult rangeReadResult = f.get(); if (!rangeReadResult.empty()) { TraceEvent(SevError, "DataOutsideTenants") .detail("Count", rangeReadFutures.size())