address pr comments

This commit is contained in:
Nim Wijetunga 2023-02-10 15:56:41 -08:00
parent de9eef72ff
commit 9e5c61e127
8 changed files with 75 additions and 56 deletions

View File

@ -196,6 +196,11 @@ Key TenantMetadata::tenantMapPrivatePrefix() {
return _prefix;
}
KeyBackedProperty<int64_t>& TenantMetadata::tenantIdPrefix() {
static KeyBackedProperty<int64_t> instance(TenantMetadata::instance().subspace.withSuffix("idPrefix"_sr));
return instance;
}
TEST_CASE("/fdbclient/libb64/base64decoder") {
Standalone<StringRef> buf = makeString(100);
for (int i = 0; i < 1000; ++i) {

View File

@ -464,7 +464,7 @@ Future<Optional<std::string>> createMetacluster(Reference<DB> db, ClusterName na
state Future<Optional<MetaclusterRegistrationEntry>> metaclusterRegistrationFuture =
MetaclusterMetadata::metaclusterRegistration().get(tr);
state Future<Void> metaClusterEmptinessCheck = managementClusterCheckEmpty(tr);
state Future<Void> metaclusterEmptinessCheck = managementClusterCheckEmpty(tr);
Optional<MetaclusterRegistrationEntry> existingRegistration = wait(metaclusterRegistrationFuture);
if (existingRegistration.present()) {
@ -479,7 +479,7 @@ Future<Optional<std::string>> createMetacluster(Reference<DB> db, ClusterName na
}
}
wait(metaClusterEmptinessCheck);
wait(metaclusterEmptinessCheck);
if (!metaclusterUid.present()) {
metaclusterUid = deterministicRandom()->randomUniqueID();
@ -488,7 +488,7 @@ Future<Optional<std::string>> createMetacluster(Reference<DB> db, ClusterName na
MetaclusterMetadata::metaclusterRegistration().set(
tr, MetaclusterRegistrationEntry(name, metaclusterUid.get()));
KeyBackedProperty<int64_t>(TenantAPI::tenantIdPrefixKey).set(tr, tenantIdPrefix);
TenantMetadata::tenantIdPrefix().set(tr, tenantIdPrefix);
wait(buggifiedCommit(tr, BUGGIFY_WITH_PROB(0.1)));
break;
@ -1278,7 +1278,7 @@ struct CreateTenantImpl {
// If the last tenant id is not present fetch the prefix from system keys and make it the prefix for the next
// allocated tenant id
if (!lastId.present()) {
Optional<int64_t> tenantIdPrefix = wait(KeyBackedProperty<int64_t>(TenantAPI::tenantIdPrefixKey).get(tr));
Optional<int64_t> tenantIdPrefix = wait(TenantMetadata::tenantIdPrefix().get(tr));
ASSERT(tenantIdPrefix.present());
lastId = tenantIdPrefix.get() << 48;
}

View File

@ -37,10 +37,6 @@ Key idToPrefix(int64_t id);
int64_t prefixToId(KeyRef prefix, EnforceValidTenantId = EnforceValidTenantId::True);
constexpr static int PREFIX_SIZE = sizeof(int64_t);
// This system keys stores the tenant id prefix that is used during metacluster/standalone cluster creation. If the key
// is not present then we will assume the prefix to be 0
const KeyRef tenantIdPrefixKey = "\xff/tenant_id_prefix"_sr;
} // namespace TenantAPI
// Represents the various states that a tenant could be in.
@ -232,6 +228,9 @@ struct TenantMetadata {
static inline auto& tenantGroupTenantIndex() { return instance().tenantGroupTenantIndex; }
static inline auto& tenantGroupMap() { return instance().tenantGroupMap; }
static inline auto& lastTenantModification() { return instance().lastTenantModification; }
// This system keys stores the tenant id prefix that is used during metacluster/standalone cluster creation. If the
// key is not present then we will assume the prefix to be 0
static KeyBackedProperty<int64_t>& tenantIdPrefix();
static Key tenantMapPrivatePrefix();
};

View File

@ -21,6 +21,7 @@
#pragma once
#include "fdbclient/ClientBooleanParams.h"
#include "fdbclient/Knobs.h"
#include "fdbclient/Tenant.h"
#include "flow/IRandom.h"
#include "flow/ThreadHelper.actor.h"
#include <algorithm>
@ -223,21 +224,17 @@ createTenantTransaction(Transaction tr, TenantMapEntry tenantEntry, ClusterType
}
ACTOR template <class Transaction>
Future<int64_t> getNextTenantId(Transaction tr, int numTenantsCreating) {
Future<int64_t> getNextTenantId(Transaction tr) {
state Optional<int64_t> lastId = wait(TenantMetadata::lastTenantId().get(tr));
if (!lastId.present()) {
// If the last tenant id is not present fetch the tenantIdPrefix (if any) and initalize the lastId
int64_t tenantIdPrefix =
wait(KeyBackedProperty<int64_t>(TenantAPI::tenantIdPrefixKey).getD(tr, Snapshot::False, 0));
int64_t tenantIdPrefix = wait(TenantMetadata::tenantIdPrefix().getD(tr, Snapshot::False, 0));
// Shift by 6 bytes to make the prefix the first two bytes of the tenant id
lastId = tenantIdPrefix << 48;
}
int64_t tenantId = lastId.get() + 1;
// Only increment the tenant Id by a large factor if there is enough tenantIds for all tenants being created
// (without overriding the tenant prefix)
if (BUGGIFY && tenantId < getMaxAllowableTenantId(lastId.get()) - numTenantsCreating) {
if (BUGGIFY) {
tenantId += deterministicRandom()->randomSkewedUInt32(1, 1e9);
tenantId = std::min(tenantId, getMaxAllowableTenantId(lastId.get()) - numTenantsCreating);
}
if (!TenantAPI::nextTenantIdPrefixMatches(lastId.get(), tenantId)) {
throw cluster_no_capacity();
@ -266,7 +263,7 @@ Future<Optional<TenantMapEntry>> createTenant(Reference<DB> db,
state Future<int64_t> tenantIdFuture;
if (generateTenantId) {
tenantIdFuture = getNextTenantId(tr, 1);
tenantIdFuture = getNextTenantId(tr);
}
if (checkExistence) {

View File

@ -132,7 +132,7 @@ private:
std::map<TenantGroupName, int>* tenantGroupNetTenantDelta) {
state Future<int64_t> tenantCountFuture =
TenantMetadata::tenantCount().getD(&ryw->getTransaction(), Snapshot::False, 0);
int64_t _nextId = wait(TenantAPI::getNextTenantId(&ryw->getTransaction(), tenants.size()));
int64_t _nextId = wait(TenantAPI::getNextTenantId(&ryw->getTransaction()));
state int64_t nextId = _nextId;
ASSERT(nextId > 0);

View File

@ -24,6 +24,8 @@
// When actually compiled (NO_INTELLISENSE), include the generated version of this file. In intellisense use the source
// version.
#include "fdbclient/FDBOptions.g.h"
#include "fdbclient/Tenant.h"
#include "fdbclient/TenantManagement.actor.h"
#include "flow/BooleanParam.h"
#if defined(NO_INTELLISENSE) && !defined(WORKLOADS_METACLUSTER_CONSISTENCY_ACTOR_G_H)
#define WORKLOADS_METACLUSTER_CONSISTENCY_ACTOR_G_H
@ -60,6 +62,7 @@ private:
int64_t tenantCount;
RangeResult systemTenantSubspaceKeys;
int64_t tenantIdPrefix;
};
ManagementClusterData managementMetadata;
@ -71,41 +74,50 @@ private:
ACTOR static Future<Void> loadManagementClusterMetadata(MetaclusterConsistencyCheck* self) {
state Reference<typename DB::TransactionT> managementTr = self->managementDb->createTransaction();
state KeyBackedRangeResult<std::pair<int64_t, TenantMapEntry>> tenantList;
state Optional<int64_t> lastTenantId;
loop {
try {
managementTr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS);
state typename transaction_future_type<typename DB::TransactionT, RangeResult>::type
systemTenantSubspaceKeysFuture = managementTr->getRange(prefixRange(TenantMetadata::subspace()), 1);
systemTenantSubspaceKeysFuture = managementTr->getRange(prefixRange(TenantMetadata::subspace()), 2);
state Optional<int64_t> tenantIdPrefix = wait(TenantMetadata::tenantIdPrefix().get(managementTr));
ASSERT(tenantIdPrefix.present());
ASSERT(tenantIdPrefix.get() >= TenantAPI::TENANT_ID_PREFIX_MIN_VALUE &&
tenantIdPrefix.get() <= TenantAPI::TENANT_ID_PREFIX_MAX_VALUE);
self->managementMetadata.tenantIdPrefix = tenantIdPrefix.get();
wait(store(self->managementMetadata.metaclusterRegistration,
MetaclusterMetadata::metaclusterRegistration().get(managementTr)) &&
store(self->managementMetadata.dataClusters,
MetaclusterAPI::listClustersTransaction(
managementTr, ""_sr, "\xff\xff"_sr, CLIENT_KNOBS->MAX_DATA_CLUSTERS + 1)) &&
store(self->managementMetadata.clusterCapacityTuples,
MetaclusterAPI::ManagementClusterMetadata::clusterCapacityIndex.getRange(
managementTr, {}, {}, CLIENT_KNOBS->MAX_DATA_CLUSTERS)) &&
store(self->managementMetadata.clusterTenantCounts,
MetaclusterAPI::ManagementClusterMetadata::clusterTenantCount.getRange(
managementTr, {}, {}, CLIENT_KNOBS->MAX_DATA_CLUSTERS)) &&
store(self->managementMetadata.clusterTenantTuples,
MetaclusterAPI::ManagementClusterMetadata::clusterTenantIndex.getRange(
managementTr, {}, {}, metaclusterMaxTenants)) &&
store(self->managementMetadata.clusterTenantGroupTuples,
MetaclusterAPI::ManagementClusterMetadata::clusterTenantGroupIndex.getRange(
managementTr, {}, {}, metaclusterMaxTenants)) &&
store(self->managementMetadata.tenantCount,
MetaclusterAPI::ManagementClusterMetadata::tenantMetadata().tenantCount.getD(
managementTr, Snapshot::False, 0)) &&
store(tenantList,
MetaclusterAPI::ManagementClusterMetadata::tenantMetadata().tenantMap.getRange(
managementTr, {}, {}, metaclusterMaxTenants)) &&
store(self->managementMetadata.tenantGroups,
MetaclusterAPI::ManagementClusterMetadata::tenantMetadata().tenantGroupMap.getRange(
managementTr, {}, {}, metaclusterMaxTenants)) &&
store(self->managementMetadata.systemTenantSubspaceKeys,
safeThreadFutureToFuture(systemTenantSubspaceKeysFuture)));
wait(
store(lastTenantId,
MetaclusterAPI::ManagementClusterMetadata::tenantMetadata().lastTenantId.get(managementTr)) &&
store(self->managementMetadata.metaclusterRegistration,
MetaclusterMetadata::metaclusterRegistration().get(managementTr)) &&
store(self->managementMetadata.dataClusters,
MetaclusterAPI::listClustersTransaction(
managementTr, ""_sr, "\xff\xff"_sr, CLIENT_KNOBS->MAX_DATA_CLUSTERS + 1)) &&
store(self->managementMetadata.clusterCapacityTuples,
MetaclusterAPI::ManagementClusterMetadata::clusterCapacityIndex.getRange(
managementTr, {}, {}, CLIENT_KNOBS->MAX_DATA_CLUSTERS)) &&
store(self->managementMetadata.clusterTenantCounts,
MetaclusterAPI::ManagementClusterMetadata::clusterTenantCount.getRange(
managementTr, {}, {}, CLIENT_KNOBS->MAX_DATA_CLUSTERS)) &&
store(self->managementMetadata.clusterTenantTuples,
MetaclusterAPI::ManagementClusterMetadata::clusterTenantIndex.getRange(
managementTr, {}, {}, metaclusterMaxTenants)) &&
store(self->managementMetadata.clusterTenantGroupTuples,
MetaclusterAPI::ManagementClusterMetadata::clusterTenantGroupIndex.getRange(
managementTr, {}, {}, metaclusterMaxTenants)) &&
store(self->managementMetadata.tenantCount,
MetaclusterAPI::ManagementClusterMetadata::tenantMetadata().tenantCount.getD(
managementTr, Snapshot::False, 0)) &&
store(tenantList,
MetaclusterAPI::ManagementClusterMetadata::tenantMetadata().tenantMap.getRange(
managementTr, {}, {}, metaclusterMaxTenants)) &&
store(self->managementMetadata.tenantGroups,
MetaclusterAPI::ManagementClusterMetadata::tenantMetadata().tenantGroupMap.getRange(
managementTr, {}, {}, metaclusterMaxTenants)) &&
store(self->managementMetadata.systemTenantSubspaceKeys,
safeThreadFutureToFuture(systemTenantSubspaceKeysFuture)));
break;
} catch (Error& e) {
@ -113,6 +125,10 @@ private:
}
}
if (lastTenantId.present()) {
ASSERT(TenantAPI::getTenantIdPrefix(lastTenantId.get()) == self->managementMetadata.tenantIdPrefix);
}
self->managementMetadata.tenantMap =
std::map<int64_t, TenantMapEntry>(tenantList.results.begin(), tenantList.results.end());
@ -120,6 +136,7 @@ private:
ASSERT_EQ(t.size(), 3);
TenantName tenantName = t.getString(1);
int64_t tenantId = t.getInt(2);
ASSERT(TenantAPI::getTenantIdPrefix(tenantId) == self->managementMetadata.tenantIdPrefix);
ASSERT(tenantName == self->managementMetadata.tenantMap[tenantId].tenantName);
self->managementMetadata.clusterTenantMap[t.getString(0)].insert(tenantId);
}
@ -241,8 +258,8 @@ private:
ASSERT(clusterItr->second.count(name));
}
// We should not be storing any data in the `\xff` tenant subspace.
ASSERT(managementMetadata.systemTenantSubspaceKeys.empty());
// The only key in the `\xff` tenant subspace should be the tenant id prefix
ASSERT(managementMetadata.systemTenantSubspaceKeys.size() == 1);
}
ACTOR static Future<Void> validateDataCluster(MetaclusterConsistencyCheck* self,
@ -257,11 +274,13 @@ private:
state TenantConsistencyCheck<IDatabase> tenantConsistencyCheck(dataDb);
wait(tenantConsistencyCheck.run());
state Optional<int64_t> lastTenantId;
loop {
try {
dataTr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS);
wait(store(dataClusterRegistration, MetaclusterMetadata::metaclusterRegistration().get(dataTr)) &&
wait(store(lastTenantId, TenantMetadata::lastTenantId().get(dataTr)) &&
store(dataClusterRegistration, MetaclusterMetadata::metaclusterRegistration().get(dataTr)) &&
store(dataClusterTenantList,
TenantMetadata::tenantMap().getRange(
dataTr, {}, {}, CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER + 1)) &&
@ -275,6 +294,10 @@ private:
}
}
if (lastTenantId.present()) {
ASSERT(TenantAPI::getTenantIdPrefix(lastTenantId.get()) == self->managementMetadata.tenantIdPrefix);
}
state std::map<int64_t, TenantMapEntry> dataClusterTenantMap(dataClusterTenantList.results.begin(),
dataClusterTenantList.results.end());
state std::map<TenantGroupName, TenantGroupEntry> dataClusterTenantGroupMap(
@ -312,6 +335,7 @@ private:
TenantMapEntry const& metaclusterEntry = self->managementMetadata.tenantMap[tenantId];
ASSERT(!entry.assignedCluster.present());
ASSERT_EQ(entry.id, metaclusterEntry.id);
ASSERT(TenantAPI::getTenantIdPrefix(entry.id) == self->managementMetadata.tenantIdPrefix);
ASSERT(entry.tenantName == metaclusterEntry.tenantName);
ASSERT_EQ(entry.tenantState, TenantState::READY);

View File

@ -65,14 +65,8 @@ struct TenantCapacityLimits : TestWorkload {
}
}
void disableFailureInjectionWorkloads(std::set<std::string>& out) const override { out.insert("Attrition"); }
Future<Void> setup(Database const& cx) override {
if (clientId == 0) {
if (g_network->isSimulated() && BUGGIFY) {
IKnobCollection::getMutableGlobalKnobCollection().setKnob(
"max_tenants_per_cluster", KnobValueRef::create(int{ deterministicRandom()->randomInt(20, 100) }));
}
return _setup(cx, this);
} else {
return Void();

View File

@ -388,7 +388,7 @@ struct TenantManagementWorkload : TestWorkload {
self->dataDb.getReference(), tenantsToCreate.begin()->first, tenantsToCreate.begin()->second)));
} else if (operationType == OperationType::MANAGEMENT_TRANSACTION) {
tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
int64_t _nextId = wait(TenantAPI::getNextTenantId(tr, tenantsToCreate.size()));
int64_t _nextId = wait(TenantAPI::getNextTenantId(tr));
int64_t nextId = _nextId;
std::vector<Future<Void>> createFutures;