From 712f40b72775b17a2c264328e6f224891dd00ed1 Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Fri, 8 Jul 2022 15:56:22 -0700 Subject: [PATCH 01/20] Add support for tenant groups --- bindings/python/tests/fdbcli_tests.py | 54 ++++- fdbcli/TenantCommands.actor.cpp | 181 +++++++++++++---- fdbcli/fdbcli.actor.cpp | 7 + fdbcli/include/fdbcli/fdbcli.actor.h | 2 + fdbclient/SystemData.cpp | 2 + fdbclient/Tenant.cpp | 13 +- fdbclient/TenantManagement.actor.cpp | 39 ++++ fdbclient/TenantSpecialKeys.cpp | 10 + fdbclient/include/fdbclient/SystemData.h | 1 + fdbclient/include/fdbclient/Tenant.h | 25 ++- .../fdbclient/TenantManagement.actor.h | 75 +++++-- .../fdbclient/TenantSpecialKeys.actor.h | 135 +++++++++++-- fdbserver/tester.actor.cpp | 8 +- .../workloads/FuzzApiCorrectness.actor.cpp | 16 +- .../TenantManagementWorkload.actor.cpp | 189 +++++++++++++++--- flow/include/flow/Arena.h | 12 +- flow/include/flow/ProtocolVersion.h | 1 + flow/include/flow/error_definitions.h | 11 +- tests/slow/SwizzledTenantManagement.toml | 2 +- tests/slow/TenantManagement.toml | 4 +- 20 files changed, 656 insertions(+), 131 deletions(-) create mode 100644 fdbclient/TenantManagement.actor.cpp diff --git a/bindings/python/tests/fdbcli_tests.py b/bindings/python/tests/fdbcli_tests.py index 12898e55cd..673557f16d 100755 --- a/bindings/python/tests/fdbcli_tests.py +++ b/bindings/python/tests/fdbcli_tests.py @@ -599,7 +599,7 @@ def tenants(logger): output = run_fdbcli_command('createtenant tenant') assert output == 'The tenant `tenant\' has been created' - output = run_fdbcli_command('createtenant tenant2') + output = run_fdbcli_command('createtenant tenant2 tenant_group=tenant_group2') assert output == 'The tenant `tenant2\' has been created' output = run_fdbcli_command('listtenants') @@ -629,6 +629,58 @@ def tenants(logger): assert('id' in json_output['tenant']) assert('prefix' in json_output['tenant']) + output = run_fdbcli_command('gettenant tenant2') + lines = output.split('\n') + assert len(lines) == 3 + assert lines[0].strip().startswith('id: ') + assert lines[1].strip().startswith('prefix: ') + assert lines[2].strip() == 'tenant group: tenant_group2' + + output = run_fdbcli_command('gettenant tenant2 JSON') + json_output = json.loads(output, strict=False) + assert(len(json_output) == 2) + assert('tenant' in json_output) + assert(json_output['type'] == 'success') + assert(len(json_output['tenant']) == 3) + assert('id' in json_output['tenant']) + assert('prefix' in json_output['tenant']) + assert(json_output['tenant']['tenant_group'] == 'tenant_group2') + + output = run_fdbcli_command('configuretenant tenant tenant_group=tenant_group1') + assert output == 'The configuration for tenant `tenant\' has been updated' + + output = run_fdbcli_command('gettenant tenant') + lines = output.split('\n') + assert len(lines) == 3 + assert lines[2].strip() == 'tenant group: tenant_group1' + + output = run_fdbcli_command('configuretenant tenant tenant_group=tenant_group1 tenant_group=tenant_group2') + assert output == 'The configuration for tenant `tenant\' has been updated' + + output = run_fdbcli_command('gettenant tenant') + lines = output.split('\n') + assert len(lines) == 3 + assert lines[2].strip() == 'tenant group: tenant_group2' + + output = run_fdbcli_command('configuretenant tenant unset tenant_group') + assert output == 'The configuration for tenant `tenant\' has been updated' + + output = run_fdbcli_command('gettenant tenant') + lines = output.split('\n') + assert len(lines) == 2 + + output = run_fdbcli_command_and_get_error('configuretenant tenant unset') + assert output == 'ERROR: `unset\' specified without a configuration parameter.' + + output = run_fdbcli_command_and_get_error('configuretenant tenant unset tenant_group=tenant_group1') + assert output == 'ERROR: unrecognized configuration parameter `tenant_group=tenant_group1\'' + + output = run_fdbcli_command_and_get_error('configuretenant tenant tenant_group') + assert output == 'ERROR: invalid configuration string `tenant_group\'. String must specify a value using `=\'.' + + output = run_fdbcli_command_and_get_error('configuretenant tenant3 tenant_group=tenant_group1') + assert output == 'ERROR: Tenant does not exist (2131)' + output = run_fdbcli_command('usetenant') assert output == 'Using the default tenant' diff --git a/fdbcli/TenantCommands.actor.cpp b/fdbcli/TenantCommands.actor.cpp index 4e272a8709..fa47b80f4b 100644 --- a/fdbcli/TenantCommands.actor.cpp +++ b/fdbcli/TenantCommands.actor.cpp @@ -35,20 +35,82 @@ namespace fdb_cli { -const KeyRangeRef tenantSpecialKeyRange(LiteralStringRef("\xff\xff/management/tenant/map/"), - LiteralStringRef("\xff\xff/management/tenant/map0")); +const KeyRangeRef tenantMapSpecialKeyRange(LiteralStringRef("\xff\xff/management/tenant/map/"), + LiteralStringRef("\xff\xff/management/tenant/map0")); +const KeyRangeRef tenantConfigSpecialKeyRange(LiteralStringRef("\xff\xff/management/tenant/configure/"), + LiteralStringRef("\xff\xff/management/tenant/configure0")); + +Optional, Optional>> +parseTenantConfiguration(std::vector const& tokens, int startIndex, bool allowUnset) { + std::map, Optional> configParams; + for (int tokenNum = startIndex; tokenNum < tokens.size(); ++tokenNum) { + Optional value; + + StringRef token = tokens[tokenNum]; + StringRef param; + if (allowUnset && token == "unset"_sr) { + if (++tokenNum == tokens.size()) { + fmt::print(stderr, "ERROR: `unset' specified without a configuration parameter.\n"); + return {}; + } + param = tokens[tokenNum]; + } else { + bool foundEquals; + param = token.eat("=", &foundEquals); + if (!foundEquals) { + fmt::print(stderr, + "ERROR: invalid configuration string `{}'. String must specify a value using `='.\n", + param.toString().c_str()); + return {}; + } + value = token; + } + + if (tokencmp(param, "tenant_group")) { + configParams[param] = value; + } else { + fmt::print(stderr, "ERROR: unrecognized configuration parameter `{}'\n", param.toString().c_str()); + return {}; + } + } + + return configParams; +} + +Key makeConfigKey(TenantNameRef tenantName, StringRef configName) { + return tenantConfigSpecialKeyRange.begin.withSuffix(Tuple().append(tenantName).append(configName).pack()); +} + +void applyConfiguration(Reference tr, + TenantNameRef tenantName, + std::map, Optional> configuration) { + for (auto [configName, value] : configuration) { + if (value.present()) { + tr->set(makeConfigKey(tenantName, configName), value.get()); + } else { + tr->clear(makeConfigKey(tenantName, configName)); + } + } +} // createtenant command ACTOR Future createTenantCommandActor(Reference db, std::vector tokens) { - if (tokens.size() != 2) { + if (tokens.size() < 2 || tokens.size() > 3) { printUsage(tokens[0]); return false; } - state Key tenantNameKey = fdb_cli::tenantSpecialKeyRange.begin.withSuffix(tokens[1]); + state Key tenantNameKey = tenantMapSpecialKeyRange.begin.withSuffix(tokens[1]); state Reference tr = db->createTransaction(); state bool doneExistenceCheck = false; + state Optional, Optional>> configuration = + parseTenantConfiguration(tokens, 2, false); + + if (!configuration.present()) { + return false; + } + loop { tr->setOption(FDBTransactionOptions::SPECIAL_KEY_SPACE_ENABLE_WRITES); try { @@ -63,25 +125,26 @@ ACTOR Future createTenantCommandActor(Reference db, std::vector } tr->set(tenantNameKey, ValueRef()); + applyConfiguration(tr, tokens[1], configuration.get()); wait(safeThreadFutureToFuture(tr->commit())); break; } catch (Error& e) { state Error err(e); if (e.code() == error_code_special_keys_api_failure) { - std::string errorMsgStr = wait(fdb_cli::getSpecialKeysFailureErrorMessage(tr)); - fprintf(stderr, "ERROR: %s\n", errorMsgStr.c_str()); + std::string errorMsgStr = wait(getSpecialKeysFailureErrorMessage(tr)); + fmt::print(stderr, "ERROR: {}\n", errorMsgStr.c_str()); return false; } wait(safeThreadFutureToFuture(tr->onError(err))); } } - printf("The tenant `%s' has been created\n", printable(tokens[1]).c_str()); + fmt::print("The tenant `{}' has been created\n", printable(tokens[1]).c_str()); return true; } CommandFactory createTenantFactory("createtenant", - CommandHelp("createtenant ", + CommandHelp("createtenant [tenant_group=]", "creates a new tenant in the cluster", "Creates a new tenant in the cluster with the specified name.")); @@ -92,7 +155,7 @@ ACTOR Future deleteTenantCommandActor(Reference db, std::vector return false; } - state Key tenantNameKey = fdb_cli::tenantSpecialKeyRange.begin.withSuffix(tokens[1]); + state Key tenantNameKey = tenantMapSpecialKeyRange.begin.withSuffix(tokens[1]); state Reference tr = db->createTransaction(); state bool doneExistenceCheck = false; @@ -115,15 +178,15 @@ ACTOR Future deleteTenantCommandActor(Reference db, std::vector } catch (Error& e) { state Error err(e); if (e.code() == error_code_special_keys_api_failure) { - std::string errorMsgStr = wait(fdb_cli::getSpecialKeysFailureErrorMessage(tr)); - fprintf(stderr, "ERROR: %s\n", errorMsgStr.c_str()); + std::string errorMsgStr = wait(getSpecialKeysFailureErrorMessage(tr)); + fmt::print(stderr, "ERROR: {}\n", errorMsgStr.c_str()); return false; } wait(safeThreadFutureToFuture(tr->onError(err))); } } - printf("The tenant `%s' has been deleted\n", printable(tokens[1]).c_str()); + fmt::print("The tenant `{}' has been deleted\n", printable(tokens[1]).c_str()); return true; } @@ -151,20 +214,20 @@ ACTOR Future listTenantsCommandActor(Reference db, std::vector< if (tokens.size() >= 3) { endTenant = tokens[2]; if (endTenant <= beginTenant) { - fprintf(stderr, "ERROR: end must be larger than begin"); + fmt::print(stderr, "ERROR: end must be larger than begin"); return false; } } if (tokens.size() == 4) { int n = 0; - if (sscanf(tokens[3].toString().c_str(), "%d%n", &limit, &n) != 1 || n != tokens[3].size()) { - fprintf(stderr, "ERROR: invalid limit %s\n", tokens[3].toString().c_str()); + if (sscanf(tokens[3].toString().c_str(), "%d%n", &limit, &n) != 1 || n != tokens[3].size() || limit <= 0) { + fmt::print(stderr, "ERROR: invalid limit `{}'\n", tokens[3].toString().c_str()); return false; } } - state Key beginTenantKey = fdb_cli::tenantSpecialKeyRange.begin.withSuffix(beginTenant); - state Key endTenantKey = fdb_cli::tenantSpecialKeyRange.begin.withSuffix(endTenant); + state Key beginTenantKey = tenantMapSpecialKeyRange.begin.withSuffix(beginTenant); + state Key endTenantKey = tenantMapSpecialKeyRange.begin.withSuffix(endTenant); state Reference tr = db->createTransaction(); loop { @@ -176,25 +239,24 @@ ACTOR Future listTenantsCommandActor(Reference db, std::vector< if (tenants.empty()) { if (tokens.size() == 1) { - printf("The cluster has no tenants\n"); + fmt::print("The cluster has no tenants\n"); } else { - printf("The cluster has no tenants in the specified range\n"); + fmt::print("The cluster has no tenants in the specified range\n"); } } int index = 0; for (auto tenant : tenants) { - printf(" %d. %s\n", - ++index, - printable(tenant.key.removePrefix(fdb_cli::tenantSpecialKeyRange.begin)).c_str()); + fmt::print( + " {}. {}\n", ++index, printable(tenant.key.removePrefix(tenantMapSpecialKeyRange.begin)).c_str()); } return true; } catch (Error& e) { state Error err(e); if (e.code() == error_code_special_keys_api_failure) { - std::string errorMsgStr = wait(fdb_cli::getSpecialKeysFailureErrorMessage(tr)); - fprintf(stderr, "ERROR: %s\n", errorMsgStr.c_str()); + std::string errorMsgStr = wait(getSpecialKeysFailureErrorMessage(tr)); + fmt::print(stderr, "ERROR: {}\n", errorMsgStr.c_str()); return false; } wait(safeThreadFutureToFuture(tr->onError(err))); @@ -217,7 +279,7 @@ ACTOR Future getTenantCommandActor(Reference db, std::vector tr = db->createTransaction(); loop { @@ -236,18 +298,24 @@ ACTOR Future getTenantCommandActor(Reference db, std::vector getTenantCommandActor(Reference db, std::vector configureTenantCommandActor(Reference db, std::vector tokens) { + if (tokens.size() < 3) { + printUsage(tokens[0]); + return false; + } + + state Optional, Optional>> configuration = + parseTenantConfiguration(tokens, 2, true); + + if (!configuration.present()) { + return false; + } + + state Reference tr = db->createTransaction(); + + loop { + tr->setOption(FDBTransactionOptions::SPECIAL_KEY_SPACE_ENABLE_WRITES); + try { + applyConfiguration(tr, tokens[1], configuration.get()); + wait(safeThreadFutureToFuture(tr->commit())); + break; + } catch (Error& e) { + state Error err(e); + if (e.code() == error_code_special_keys_api_failure) { + std::string errorMsgStr = wait(getSpecialKeysFailureErrorMessage(tr)); + fmt::print(stderr, "ERROR: {}\n", errorMsgStr.c_str()); + return false; + } + wait(safeThreadFutureToFuture(tr->onError(err))); + } + } + + fmt::print("The configuration for tenant `{}' has been updated\n", printable(tokens[1]).c_str()); + return true; +} + +CommandFactory configureTenantFactory( + "configuretenant", + CommandHelp("configuretenant <[unset] tenant_group[=]> ...", + "updates the configuration for a tenant", + "Updates the configuration for a tenant. Use `tenant_group=' to change the tenant group " + "that a tenant is assigned to or `unset tenant_group' to remove a tenant from its tenant group.")); + // renametenant command ACTOR Future renameTenantCommandActor(Reference db, std::vector tokens) { if (tokens.size() != 3) { @@ -296,7 +408,8 @@ ACTOR Future renameTenantCommandActor(Reference db, std::vector } wait(safeThreadFutureToFuture(TenantAPI::renameTenant(db, tokens[1], tokens[2]))); - printf("The tenant `%s' has been renamed to `%s'\n", printable(tokens[1]).c_str(), printable(tokens[2]).c_str()); + fmt::print( + "The tenant `{}' has been renamed to `{}'\n", printable(tokens[1]).c_str(), printable(tokens[2]).c_str()); return true; } @@ -304,6 +417,6 @@ CommandFactory renameTenantFactory( "renametenant", CommandHelp( "renametenant ", - "renames a tenant in the cluster.", + "renames a tenant in the cluster", "Renames a tenant in the cluster. The old name must exist and the new name must not exist in the cluster.")); } // namespace fdb_cli diff --git a/fdbcli/fdbcli.actor.cpp b/fdbcli/fdbcli.actor.cpp index 5c93a9538f..e772af6619 100644 --- a/fdbcli/fdbcli.actor.cpp +++ b/fdbcli/fdbcli.actor.cpp @@ -1918,6 +1918,13 @@ ACTOR Future cli(CLIOptions opt, LineNoise* plinenoise) { continue; } + if (tokencmp(tokens[0], "configuretenant")) { + bool _result = wait(makeInterruptable(configureTenantCommandActor(db, tokens))); + if (!_result) + is_error = true; + continue; + } + if (tokencmp(tokens[0], "renametenant")) { bool _result = wait(makeInterruptable(renameTenantCommandActor(db, tokens))); if (!_result) diff --git a/fdbcli/include/fdbcli/fdbcli.actor.h b/fdbcli/include/fdbcli/fdbcli.actor.h index 25e4bcdfca..82527a15aa 100644 --- a/fdbcli/include/fdbcli/fdbcli.actor.h +++ b/fdbcli/include/fdbcli/fdbcli.actor.h @@ -157,6 +157,8 @@ ACTOR Future configureCommandActor(Reference db, std::vector tokens, LineNoise* linenoise, Future warn); +// configuretenant command +ACTOR Future configureTenantCommandActor(Reference db, std::vector tokens); // consistency command ACTOR Future consistencyCheckCommandActor(Reference tr, std::vector tokens, diff --git a/fdbclient/SystemData.cpp b/fdbclient/SystemData.cpp index 2fb2bf7180..0d5451b8c0 100644 --- a/fdbclient/SystemData.cpp +++ b/fdbclient/SystemData.cpp @@ -1559,6 +1559,8 @@ const KeyRef tenantMapPrefix = tenantMapKeys.begin; const KeyRef tenantMapPrivatePrefix = "\xff\xff/tenantMap/"_sr; const KeyRef tenantLastIdKey = "\xff/tenantLastId/"_sr; const KeyRef tenantDataPrefixKey = "\xff/tenantDataPrefix"_sr; +const KeyRangeRef tenantGroupTenantIndexKeys("\xff/tenant/tenantGroup/tenantMap/"_sr, + "\xff/tenant/tenantGroup/tenantMap0"_sr); // for tests void testSSISerdes(StorageServerInterface const& ssi) { diff --git a/fdbclient/Tenant.cpp b/fdbclient/Tenant.cpp index 1071059114..1b989aa313 100644 --- a/fdbclient/Tenant.cpp +++ b/fdbclient/Tenant.cpp @@ -35,7 +35,7 @@ int64_t TenantMapEntry::prefixToId(KeyRef prefix) { return id; } -void TenantMapEntry::initPrefix(KeyRef subspace) { +void TenantMapEntry::setSubspace(KeyRef subspace) { ASSERT(id >= 0); prefix = makeString(8 + subspace.size()); uint8_t* data = mutateString(prefix); @@ -48,7 +48,16 @@ void TenantMapEntry::initPrefix(KeyRef subspace) { TenantMapEntry::TenantMapEntry() : id(-1) {} TenantMapEntry::TenantMapEntry(int64_t id, KeyRef subspace) : id(id) { - initPrefix(subspace); + setSubspace(subspace); +} + +TenantMapEntry::TenantMapEntry(int64_t id, KeyRef subspace, Optional tenantGroup) + : id(id), tenantGroup(tenantGroup) { + setSubspace(subspace); +} + +bool TenantMapEntry::matchesConfiguration(TenantMapEntry const& other) const { + return tenantGroup == other.tenantGroup; } TEST_CASE("/fdbclient/TenantMapEntry/Serialization") { diff --git a/fdbclient/TenantManagement.actor.cpp b/fdbclient/TenantManagement.actor.cpp new file mode 100644 index 0000000000..2d458be02c --- /dev/null +++ b/fdbclient/TenantManagement.actor.cpp @@ -0,0 +1,39 @@ +/* + * TenantManagement.actor.cpp + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2013-2022 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include "fdbclient/SystemData.h" +#include "fdbclient/TenantManagement.actor.h" +#include "fdbclient/Tuple.h" +#include "flow/actorcompiler.h" // has to be last include + +namespace TenantAPI { + +Key getTenantGroupIndexKey(TenantGroupNameRef tenantGroup, Optional tenant) { + Tuple tuple; + tuple.append(tenantGroup); + if (tenant.present()) { + tuple.append(tenant.get()); + } + return tenantGroupTenantIndexKeys.begin.withSuffix(tuple.pack()); +} + +} // namespace TenantAPI \ No newline at end of file diff --git a/fdbclient/TenantSpecialKeys.cpp b/fdbclient/TenantSpecialKeys.cpp index 05d51b7e93..5570816684 100644 --- a/fdbclient/TenantSpecialKeys.cpp +++ b/fdbclient/TenantSpecialKeys.cpp @@ -31,3 +31,13 @@ const KeyRangeRef TenantRangeImpl::submoduleRange = KeyRangeRef(""_sr, "\ template <> const KeyRangeRef TenantRangeImpl::mapSubRange = KeyRangeRef("tenant_map/"_sr, "tenant_map0"_sr); + +template <> +bool TenantRangeImpl::subRangeIntersects(KeyRangeRef subRange, KeyRangeRef range) { + return subRange.intersects(range); +} + +template <> +bool TenantRangeImpl::subRangeIntersects(KeyRangeRef subRange, KeyRangeRef range) { + return subRange == mapSubRange; +} \ No newline at end of file diff --git a/fdbclient/include/fdbclient/SystemData.h b/fdbclient/include/fdbclient/SystemData.h index 088d137d5c..1b7db4ded2 100644 --- a/fdbclient/include/fdbclient/SystemData.h +++ b/fdbclient/include/fdbclient/SystemData.h @@ -665,6 +665,7 @@ extern const KeyRef tenantMapPrefix; extern const KeyRef tenantMapPrivatePrefix; extern const KeyRef tenantLastIdKey; extern const KeyRef tenantDataPrefixKey; +extern const KeyRangeRef tenantGroupTenantIndexKeys; #pragma clang diagnostic pop diff --git a/fdbclient/include/fdbclient/Tenant.h b/fdbclient/include/fdbclient/Tenant.h index baac9f4543..67ad076613 100644 --- a/fdbclient/include/fdbclient/Tenant.h +++ b/fdbclient/include/fdbclient/Tenant.h @@ -28,6 +28,8 @@ typedef StringRef TenantNameRef; typedef Standalone TenantName; +typedef StringRef TenantGroupNameRef; +typedef Standalone TenantGroupName; struct TenantMapEntry { constexpr static FileIdentifier file_identifier = 12247338; @@ -37,21 +39,23 @@ struct TenantMapEntry { int64_t id; Key prefix; + Optional tenantGroup; constexpr static int ROOT_PREFIX_SIZE = sizeof(id); -private: - void initPrefix(KeyRef subspace); - public: TenantMapEntry(); TenantMapEntry(int64_t id, KeyRef subspace); + TenantMapEntry(int64_t id, KeyRef subspace, Optional tenantGroup); - Value encode() const { return ObjectWriter::toValue(*this, IncludeVersion(ProtocolVersion::withTenants())); } + void setSubspace(KeyRef subspace); + bool matchesConfiguration(TenantMapEntry const& other) const; + + Value encode() const { return ObjectWriter::toValue(*this, IncludeVersion(ProtocolVersion::withTenantGroups())); } static TenantMapEntry decode(ValueRef const& value) { TenantMapEntry entry; - ObjectReader reader(value.begin(), IncludeVersion(ProtocolVersion::withTenants())); + ObjectReader reader(value.begin(), IncludeVersion()); reader.deserialize(entry); return entry; } @@ -60,16 +64,21 @@ public: void serialize(Ar& ar) { KeyRef subspace; if (ar.isDeserializing) { - serializer(ar, id, subspace); + if (ar.protocolVersion().hasTenantGroups()) { + serializer(ar, id, subspace, tenantGroup); + } else { + serializer(ar, id, subspace); + } + if (id >= 0) { - initPrefix(subspace); + setSubspace(subspace); } } else { ASSERT(prefix.size() >= 8 || (prefix.empty() && id == -1)); if (!prefix.empty()) { subspace = prefix.substr(0, prefix.size() - 8); } - serializer(ar, id, subspace); + serializer(ar, id, subspace, tenantGroup); } } }; diff --git a/fdbclient/include/fdbclient/TenantManagement.actor.h b/fdbclient/include/fdbclient/TenantManagement.actor.h index d4c0c3464d..cd5f753fbb 100644 --- a/fdbclient/include/fdbclient/TenantManagement.actor.h +++ b/fdbclient/include/fdbclient/TenantManagement.actor.h @@ -32,6 +32,9 @@ #include "flow/actorcompiler.h" // has to be last include namespace TenantAPI { + +Key getTenantGroupIndexKey(TenantGroupNameRef tenantGroup, Optional tenant); + ACTOR template Future> tryGetTenantTransaction(Transaction tr, TenantName name) { state Key tenantMapKey = name.withPrefix(tenantMapPrefix); @@ -83,16 +86,20 @@ Future getTenant(Reference db, TenantName name) { // 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, int64_t tenantId) { +Future> createTenantTransaction(Transaction tr, + TenantNameRef name, + TenantMapEntry tenantEntry) { state Key tenantMapKey = name.withPrefix(tenantMapPrefix); + ASSERT(tenantEntry.id >= 0); + if (name.startsWith("\xff"_sr)) { throw invalid_tenant_name(); } tr->setOption(FDBTransactionOptions::RAW_ACCESS); - state Future> tenantEntryFuture = tryGetTenantTransaction(tr, name); + state Future> existingEntryFuture = tryGetTenantTransaction(tr, name); state typename transaction_future_type>::type tenantDataPrefixFuture = tr->get(tenantDataPrefixKey); state typename transaction_future_type>::type tenantModeFuture = @@ -104,9 +111,9 @@ Future> createTenantTransaction(Transaction tr, throw tenants_disabled(); } - Optional tenantEntry = wait(tenantEntryFuture); - if (tenantEntry.present()) { - return std::make_pair(tenantEntry.get(), false); + Optional existingEntry = wait(existingEntryFuture); + if (existingEntry.present()) { + return std::make_pair(existingEntry.get(), false); } Optional tenantDataPrefix = wait(safeThreadFutureToFuture(tenantDataPrefixFuture)); @@ -121,30 +128,38 @@ Future> createTenantTransaction(Transaction tr, throw client_invalid_operation(); } - state TenantMapEntry newTenant(tenantId, tenantDataPrefix.present() ? (KeyRef)tenantDataPrefix.get() : ""_sr); + tenantEntry.setSubspace(tenantDataPrefix.orDefault(""_sr)); state typename transaction_future_type::type prefixRangeFuture = - tr->getRange(prefixRange(newTenant.prefix), 1); + tr->getRange(prefixRange(tenantEntry.prefix), 1); RangeResult contents = wait(safeThreadFutureToFuture(prefixRangeFuture)); if (!contents.empty()) { throw tenant_prefix_allocator_conflict(); } - tr->set(tenantMapKey, newTenant.encode()); + tr->set(tenantMapKey, tenantEntry.encode()); + if (tenantEntry.tenantGroup.present()) { + tr->set(getTenantGroupIndexKey(tenantEntry.tenantGroup.get(), name), ""_sr); + } - return std::make_pair(newTenant, true); + return std::make_pair(tenantEntry, true); } ACTOR template -Future createTenant(Reference db, TenantName name) { +Future createTenant(Reference db, TenantName name, TenantMapEntry tenantEntry = TenantMapEntry()) { state Reference tr = db->createTransaction(); state bool firstTry = true; + state bool generateTenantId = tenantEntry.id < 0; loop { try { tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); tr->setOption(FDBTransactionOptions::LOCK_AWARE); - state typename DB::TransactionT::template FutureT> lastIdFuture = tr->get(tenantLastIdKey); + + state typename DB::TransactionT::template FutureT> lastIdFuture; + if (generateTenantId) { + lastIdFuture = tr->get(tenantLastIdKey); + } if (firstTry) { Optional entry = wait(tryGetTenantTransaction(tr, name)); @@ -155,18 +170,24 @@ Future createTenant(Reference db, TenantName name) { firstTry = false; } - 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 (generateTenantId) { + Optional lastIdVal = wait(safeThreadFutureToFuture(lastIdFuture)); + tenantEntry.id = lastIdVal.present() ? TenantMapEntry::prefixToId(lastIdVal.get()) + 1 : 0; + tr->set(tenantLastIdKey, TenantMapEntry::idToPrefix(tenantEntry.id)); + } - wait(buggifiedCommit(tr, BUGGIFY_WITH_PROB(0.1))); + state std::pair newTenant = wait(createTenantTransaction(tr, name, tenantEntry)); - TraceEvent("CreatedTenant") - .detail("Tenant", name) - .detail("TenantId", newTenant.first.id) - .detail("Prefix", newTenant.first.prefix) - .detail("Version", tr->getCommittedVersion()); + if (newTenant.second) { + wait(buggifiedCommit(tr, BUGGIFY_WITH_PROB(0.1))); + + TraceEvent("CreatedTenant") + .detail("Tenant", name) + .detail("TenantId", newTenant.first.id) + .detail("Prefix", newTenant.first.prefix) + .detail("TenantGroup", tenantEntry.tenantGroup) + .detail("Version", tr->getCommittedVersion()); + } return newTenant.first; } catch (Error& e) { @@ -194,6 +215,9 @@ Future deleteTenantTransaction(Transaction tr, TenantNameRef name) { } tr->clear(tenantMapKey); + if (tenantEntry.get().tenantGroup.present()) { + tr->clear(getTenantGroupIndexKey(tenantEntry.get().tenantGroup.get(), name)); + } return Void(); } @@ -228,6 +252,15 @@ Future deleteTenant(Reference db, TenantName name) { } } +// This should only be called from a transaction that has already confirmed that the cluster entry +// is present. The updatedEntry should use the existing entry and modify only those fields that need +// to be changed. +template +void configureTenantTransaction(Transaction tr, TenantNameRef tenantName, TenantMapEntry tenantEntry) { + tr->setOption(FDBTransactionOptions::RAW_ACCESS); + tr->set(tenantName.withPrefix(tenantMapPrefix), tenantEntry.encode()); +} + ACTOR template Future> listTenantsTransaction(Transaction tr, TenantNameRef begin, diff --git a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h index 9cde887343..b61b3a28b3 100644 --- a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h +++ b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h @@ -31,13 +31,16 @@ #include "fdbclient/DatabaseContext.h" #include "fdbclient/SpecialKeySpace.actor.h" #include "fdbclient/TenantManagement.actor.h" +#include "fdbclient/Tuple.h" #include "flow/Arena.h" #include "flow/UnitTest.h" #include "flow/actorcompiler.h" // This must be the last #include. -template +template class TenantRangeImpl : public SpecialKeyRangeRWImpl { private: + static bool subRangeIntersects(KeyRangeRef subRange, KeyRangeRef range); + static KeyRangeRef removePrefix(KeyRangeRef range, KeyRef prefix, KeyRef defaultEnd) { KeyRef begin = range.begin.removePrefix(prefix); KeyRef end; @@ -52,15 +55,14 @@ private: static KeyRef withTenantMapPrefix(KeyRef key, Arena& ar) { int keySize = SpecialKeySpace::getModuleRange(SpecialKeySpace::MODULE::MANAGEMENT).begin.size() + - TenantRangeImpl::submoduleRange.begin.size() + TenantRangeImpl::mapSubRange.begin.size() + - key.size(); + submoduleRange.begin.size() + mapSubRange.begin.size() + key.size(); KeyRef prefixedKey = makeString(keySize, ar); uint8_t* mutableKey = mutateString(prefixedKey); mutableKey = SpecialKeySpace::getModuleRange(SpecialKeySpace::MODULE::MANAGEMENT).begin.copyTo(mutableKey); - mutableKey = TenantRangeImpl::submoduleRange.begin.copyTo(mutableKey); - mutableKey = TenantRangeImpl::mapSubRange.begin.copyTo(mutableKey); + mutableKey = submoduleRange.begin.copyTo(mutableKey); + mutableKey = mapSubRange.begin.copyTo(mutableKey); key.copyTo(mutableKey); return prefixedKey; @@ -77,6 +79,9 @@ private: json_spirit::mObject tenantEntry; tenantEntry["id"] = tenant.second.id; tenantEntry["prefix"] = tenant.second.prefix.toString(); + if (tenant.second.tenantGroup.present()) { + tenantEntry["tenant_group"] = tenant.second.tenantGroup.get().toString(); + } std::string tenantEntryString = json_spirit::write_string(json_spirit::mValue(tenantEntry)); ValueRef tenantEntryBytes(results->arena(), tenantEntryString); results->push_back(results->arena(), @@ -86,20 +91,21 @@ private: return Void(); } - ACTOR static Future getTenantRange(ReadYourWritesTransaction* ryw, - KeyRangeRef kr, - GetRangeLimits limitsHint) { + ACTOR template + static Future getTenantRange(ReadYourWritesTransaction* ryw, + KeyRangeRef kr, + GetRangeLimits limitsHint) { state RangeResult results; kr = kr.removePrefix(SpecialKeySpace::getModuleRange(SpecialKeySpace::MODULE::MANAGEMENT).begin) - .removePrefix(TenantRangeImpl::submoduleRange.begin); + .removePrefix(TenantRangeImpl::submoduleRange.begin); - if (kr.intersects(TenantRangeImpl::mapSubRange)) { + if (kr.intersects(TenantRangeImpl::mapSubRange)) { GetRangeLimits limits = limitsHint; limits.decrement(results); wait(getTenantList( ryw, - removePrefix(kr & TenantRangeImpl::mapSubRange, TenantRangeImpl::mapSubRange.begin, "\xff"_sr), + removePrefix(kr & TenantRangeImpl::mapSubRange, TenantRangeImpl::mapSubRange.begin, "\xff"_sr), &results, limits)); } @@ -107,14 +113,57 @@ private: return results; } - ACTOR static Future createTenants(ReadYourWritesTransaction* ryw, std::vector tenants) { + static void applyTenantConfig(ReadYourWritesTransaction* ryw, + TenantNameRef tenantName, + std::vector, Optional>> configEntries, + TenantMapEntry* tenantEntry) { + + std::vector, Optional>>::iterator configItr; + for (configItr = configEntries.begin(); configItr != configEntries.end(); ++configItr) { + if (configItr->first == "tenant_group"_sr) { + tenantEntry->tenantGroup = configItr->second; + } else { + TraceEvent(SevWarn, "InvalidTenantConfig") + .detail("TenantName", tenantName) + .detail("ConfigName", configItr->first); + ryw->setSpecialKeySpaceErrorMsg( + ManagementAPIError::toJsonString(false, + "set tenant configuration", + format("invalid tenant configuration option `%s' for tenant `%s'", + configItr->first.toString().c_str(), + tenantName.toString().c_str()))); + throw special_keys_api_failure(); + } + } + } + + ACTOR static Future createTenant( + ReadYourWritesTransaction* ryw, + TenantNameRef tenantName, + Optional, Optional>>> configMutations, + int64_t tenantId) { + state TenantMapEntry tenantEntry; + tenantEntry.id = tenantId; + + if (configMutations.present()) { + applyTenantConfig(ryw, tenantName, configMutations.get(), &tenantEntry); + } + + std::pair entry = + wait(TenantAPI::createTenantTransaction(&ryw->getTransaction(), tenantName, tenantEntry)); + + return Void(); + } + + ACTOR static Future createTenants( + ReadYourWritesTransaction* ryw, + std::map, Optional>>>> 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(TenantAPI::createTenantTransaction(&ryw->getTransaction(), tenant, ++previousId))); + for (auto const& [tenant, config] : tenants) { + createFutures.push_back(createTenant(ryw, tenant, config, ++previousId)); } ryw->getTransaction().set(tenantLastIdKey, TenantMapEntry::idToPrefix(previousId)); @@ -122,6 +171,18 @@ private: return Void(); } + ACTOR static Future changeTenantConfig( + ReadYourWritesTransaction* ryw, + TenantName tenantName, + std::vector, Optional>> configEntries) { + state TenantMapEntry tenantEntry = wait(TenantAPI::getTenantTransaction(&ryw->getTransaction(), tenantName)); + + applyTenantConfig(ryw, tenantName, configEntries, &tenantEntry); + TenantAPI::configureTenantTransaction(&ryw->getTransaction(), tenantName, tenantEntry); + + return Void(); + } + ACTOR static Future deleteTenantRange(ReadYourWritesTransaction* ryw, TenantName beginTenant, TenantName endTenant) { @@ -147,15 +208,19 @@ private: } public: + // These ranges vary based on the template parameter const static KeyRangeRef submoduleRange; const static KeyRangeRef mapSubRange; + // These sub-ranges should only be used if HasSubRanges=true + const inline static KeyRangeRef configureSubRange = KeyRangeRef("configure/"_sr, "configure0"_sr); + explicit TenantRangeImpl(KeyRangeRef kr) : SpecialKeyRangeRWImpl(kr) {} Future getRange(ReadYourWritesTransaction* ryw, KeyRangeRef kr, GetRangeLimits limitsHint) const override { - return getTenantRange(ryw, kr, limitsHint); + return getTenantRange(ryw, kr, limitsHint); } Future> commit(ReadYourWritesTransaction* ryw) override { @@ -163,6 +228,7 @@ public: std::vector> tenantManagementFutures; std::vector>> mapMutations; + std::map, Optional>>> configMutations; for (auto range : ranges) { if (!range.value().first) { @@ -174,25 +240,53 @@ public: .removePrefix(SpecialKeySpace::getModuleRange(SpecialKeySpace::MODULE::MANAGEMENT).begin) .removePrefix(submoduleRange.begin); - if (mapSubRange.intersects(adjustedRange)) { + if (subRangeIntersects(mapSubRange, adjustedRange)) { adjustedRange = mapSubRange & adjustedRange; adjustedRange = removePrefix(adjustedRange, mapSubRange.begin, "\xff"_sr); mapMutations.push_back(std::make_pair(adjustedRange, range.value().second)); + } else if (subRangeIntersects(configureSubRange, adjustedRange) && adjustedRange.singleKeyRange()) { + StringRef configTupleStr = adjustedRange.begin.removePrefix(configureSubRange.begin); + try { + Tuple tuple = Tuple::unpack(configTupleStr); + if (tuple.size() != 2) { + throw invalid_tuple_index(); + } + configMutations[tuple.getString(0)].push_back( + std::make_pair(tuple.getString(1), range.value().second)); + } catch (Error& e) { + TraceEvent(SevWarn, "InvalidTenantConfigurationKey").error(e).detail("Key", adjustedRange.begin); + ryw->setSpecialKeySpaceErrorMsg(ManagementAPIError::toJsonString( + false, "configure tenant", "invalid tenant configuration key")); + throw special_keys_api_failure(); + } } } - std::vector tenantsToCreate; + std::map, Optional>>>> tenantsToCreate; for (auto mapMutation : mapMutations) { TenantNameRef tenantName = mapMutation.first.begin; if (mapMutation.second.present()) { - tenantsToCreate.push_back(tenantName); + Optional, Optional>>> createMutations; + auto itr = configMutations.find(tenantName); + if (itr != configMutations.end()) { + createMutations = itr->second; + configMutations.erase(itr); + } + tenantsToCreate[tenantName] = createMutations; } else { // For a single key clear, just issue the delete if (mapMutation.first.singleKeyRange()) { tenantManagementFutures.push_back( TenantAPI::deleteTenantTransaction(&ryw->getTransaction(), tenantName)); + + // Configuration changes made to a deleted tenant are discarded + configMutations.erase(tenantName); } else { tenantManagementFutures.push_back(deleteTenantRange(ryw, tenantName, mapMutation.first.end)); + + // Configuration changes made to a deleted tenant are discarded + configMutations.erase(configMutations.lower_bound(tenantName), + configMutations.lower_bound(mapMutation.first.end)); } } } @@ -200,6 +294,9 @@ public: if (!tenantsToCreate.empty()) { tenantManagementFutures.push_back(createTenants(ryw, tenantsToCreate)); } + for (auto configMutation : configMutations) { + tenantManagementFutures.push_back(changeTenantConfig(ryw, configMutation.first, configMutation.second)); + } return tag(waitForAll(tenantManagementFutures), Optional()); } diff --git a/fdbserver/tester.actor.cpp b/fdbserver/tester.actor.cpp index b7fd85c554..c4bfe1e5fb 100644 --- a/fdbserver/tester.actor.cpp +++ b/fdbserver/tester.actor.cpp @@ -1629,8 +1629,12 @@ ACTOR Future runTests(Reference> tenantFutures; for (auto tenant : tenantsToCreate) { - TraceEvent("CreatingTenant").detail("Tenant", tenant); - tenantFutures.push_back(success(TenantAPI::createTenant(cx.getReference(), tenant))); + TenantMapEntry entry; + if (deterministicRandom()->coinflip()) { + entry.tenantGroup = "TestTenantGroup"_sr; + } + TraceEvent("CreatingTenant").detail("Tenant", tenant).detail("TenantGroup", entry.tenantGroup); + tenantFutures.push_back(success(TenantAPI::createTenant(cx.getReference(), tenant, entry))); } wait(waitForAll(tenantFutures)); diff --git a/fdbserver/workloads/FuzzApiCorrectness.actor.cpp b/fdbserver/workloads/FuzzApiCorrectness.actor.cpp index 390ff6f295..27000783b7 100644 --- a/fdbserver/workloads/FuzzApiCorrectness.actor.cpp +++ b/fdbserver/workloads/FuzzApiCorrectness.actor.cpp @@ -131,6 +131,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { std::vector> tenants; std::set createdTenants; int numTenants; + int numTenantGroups; // Map from tenant number to key prefix std::map keyPrefixes; @@ -154,6 +155,9 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { int maxTenants = getOption(options, "numTenants"_sr, 4); numTenants = deterministicRandom()->randomInt(0, maxTenants + 1); + int maxTenantGroups = getOption(options, "numTenantGroups"_sr, numTenants); + numTenantGroups = deterministicRandom()->randomInt(0, maxTenantGroups + 1); + // See https://github.com/apple/foundationdb/issues/2424 if (BUGGIFY) { enableBuggify(true, BuggifyType::Client); @@ -206,6 +210,14 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { std::string description() const override { return "FuzzApiCorrectness"; } static TenantName getTenant(int num) { return TenantNameRef(format("tenant_%d", num)); } + Optional getTenantGroup(int num) { + int groupNum = num % (numTenantGroups + 1); + if (groupNum == numTenantGroups - 1) { + return Optional(); + } else { + return TenantGroupNameRef(format("tenantgroup_%d", groupNum)); + } + } bool canUseTenant(Optional tenant) { return !tenant.present() || createdTenants.count(tenant.get()); } Future setup(Database const& cx) override { @@ -226,7 +238,9 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { // The last tenant will not be created if (i < self->numTenants) { - tenantFutures.push_back(::success(TenantAPI::createTenant(cx.getReference(), tenantName))); + TenantMapEntry entry; + entry.tenantGroup = self->getTenantGroup(i); + tenantFutures.push_back(::success(TenantAPI::createTenant(cx.getReference(), tenantName, entry))); self->createdTenants.insert(tenantName); } } diff --git a/fdbserver/workloads/TenantManagementWorkload.actor.cpp b/fdbserver/workloads/TenantManagementWorkload.actor.cpp index c57c3da4ad..14422afa16 100644 --- a/fdbserver/workloads/TenantManagementWorkload.actor.cpp +++ b/fdbserver/workloads/TenantManagementWorkload.actor.cpp @@ -32,15 +32,17 @@ #include "flow/actorcompiler.h" // This must be the last #include. struct TenantManagementWorkload : TestWorkload { - struct TenantState { + struct TenantData { int64_t id; + Optional tenantGroup; bool empty; - TenantState() : id(-1), empty(true) {} - TenantState(int64_t id, bool empty) : id(id), empty(empty) {} + TenantData() : id(-1), empty(true) {} + TenantData(int64_t id, Optional tenantGroup, bool empty) + : id(id), tenantGroup(tenantGroup), empty(empty) {} }; - std::map createdTenants; + std::map createdTenants; int64_t maxId = -1; Key tenantSubspace; @@ -53,8 +55,12 @@ struct TenantManagementWorkload : TestWorkload { const Key specialKeysTenantMapPrefix = SpecialKeySpace::getModuleRange(SpecialKeySpace::MODULE::MANAGEMENT) .begin.withSuffix(TenantRangeImpl::submoduleRange.begin) .withSuffix(TenantRangeImpl::mapSubRange.begin); + const Key specialKeysTenantConfigPrefix = SpecialKeySpace::getModuleRange(SpecialKeySpace::MODULE::MANAGEMENT) + .begin.withSuffix(TenantRangeImpl::submoduleRange.begin) + .withSuffix(TenantRangeImpl::configureSubRange.begin); int maxTenants; + int maxTenantGroups; double testDuration; enum class OperationType { SPECIAL_KEYS, MANAGEMENT_DATABASE, MANAGEMENT_TRANSACTION }; @@ -72,6 +78,7 @@ struct TenantManagementWorkload : TestWorkload { TenantManagementWorkload(WorkloadContext const& wcx) : TestWorkload(wcx) { maxTenants = std::min(1e8 - 1, getOption(options, "maxTenants"_sr, 1000)); + maxTenantGroups = std::min(2 * maxTenants, getOption(options, "maxTenantGroups"_sr, 20)); testDuration = getOption(options, "testDuration"_sr, 60.0); localTenantNamePrefix = format("%stenant_%d_", tenantNamePrefix.toString().c_str(), clientId); @@ -132,6 +139,15 @@ struct TenantManagementWorkload : TestWorkload { return tenant; } + Optional chooseTenantGroup() { + Optional tenantGroup; + if (deterministicRandom()->coinflip()) { + tenantGroup = + TenantGroupNameRef(format("tenantgroup%08d", deterministicRandom()->randomInt(0, maxTenantGroups))); + } + return tenantGroup; + } + ACTOR Future createTenant(Database cx, TenantManagementWorkload* self) { state OperationType operationType = TenantManagementWorkload::randomOperationType(); int numTenants = 1; @@ -144,10 +160,12 @@ struct TenantManagementWorkload : TestWorkload { state bool alreadyExists = false; state bool hasSystemTenant = false; - state std::set tenantsToCreate; + state std::map tenantsToCreate; for (int i = 0; i < numTenants; ++i) { TenantName tenant = self->chooseTenantName(true); - tenantsToCreate.insert(tenant); + TenantMapEntry entry; + entry.tenantGroup = self->chooseTenantGroup(); + tenantsToCreate[tenant] = entry; alreadyExists = alreadyExists || self->createdTenants.count(tenant); hasSystemTenant = hasSystemTenant || tenant.startsWith("\xff"_sr); @@ -159,13 +177,19 @@ struct TenantManagementWorkload : TestWorkload { try { if (operationType == OperationType::SPECIAL_KEYS) { tr->setOption(FDBTransactionOptions::SPECIAL_KEY_SPACE_ENABLE_WRITES); - for (auto tenant : tenantsToCreate) { + for (auto [tenant, entry] : tenantsToCreate) { tr->set(self->specialKeysTenantMapPrefix.withSuffix(tenant), ""_sr); + if (entry.tenantGroup.present()) { + tr->set(self->specialKeysTenantConfigPrefix.withSuffix( + Tuple().append(tenant).append("tenant_group"_sr).pack()), + entry.tenantGroup.get()); + } } wait(tr->commit()); } else if (operationType == OperationType::MANAGEMENT_DATABASE) { ASSERT(tenantsToCreate.size() == 1); - wait(success(TenantAPI::createTenant(cx.getReference(), *tenantsToCreate.begin()))); + wait(success(TenantAPI::createTenant( + cx.getReference(), tenantsToCreate.begin()->first, tenantsToCreate.begin()->second))); } else { tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); @@ -173,8 +197,9 @@ struct TenantManagementWorkload : TestWorkload { int64_t previousId = lastIdVal.present() ? TenantMapEntry::prefixToId(lastIdVal.get()) : -1; std::vector> createFutures; - for (auto tenant : tenantsToCreate) { - createFutures.push_back(success(TenantAPI::createTenantTransaction(tr, tenant, ++previousId))); + for (auto [tenant, entry] : tenantsToCreate) { + entry.id = ++previousId; + createFutures.push_back(success(TenantAPI::createTenantTransaction(tr, tenant, entry))); } tr->set(tenantLastIdKey, TenantMapEntry::idToPrefix(previousId)); wait(waitForAll(createFutures)); @@ -187,26 +212,29 @@ struct TenantManagementWorkload : TestWorkload { ASSERT(!hasSystemTenant); - state std::set::iterator tenantItr; + state std::map::iterator tenantItr; for (tenantItr = tenantsToCreate.begin(); tenantItr != tenantsToCreate.end(); ++tenantItr) { - if (self->createdTenants.count(*tenantItr)) { + if (self->createdTenants.count(tenantItr->first)) { continue; } - state Optional entry = wait(TenantAPI::tryGetTenant(cx.getReference(), *tenantItr)); + state Optional entry = + wait(TenantAPI::tryGetTenant(cx.getReference(), tenantItr->first)); ASSERT(entry.present()); ASSERT(entry.get().id > self->maxId); ASSERT(entry.get().prefix.startsWith(self->tenantSubspace)); + ASSERT(entry.get().tenantGroup == tenantItr->second.tenantGroup); self->maxId = entry.get().id; - self->createdTenants[*tenantItr] = TenantState(entry.get().id, true); + self->createdTenants[tenantItr->first] = + TenantData(entry.get().id, tenantItr->second.tenantGroup, true); state bool insertData = deterministicRandom()->random01() < 0.5; if (insertData) { - state Transaction insertTr(cx, *tenantItr); + state Transaction insertTr(cx, tenantItr->first); loop { try { - insertTr.set(self->keyName, *tenantItr); + insertTr.set(self->keyName, tenantItr->first); wait(insertTr.commit()); break; } catch (Error& e) { @@ -214,7 +242,7 @@ struct TenantManagementWorkload : TestWorkload { } } - self->createdTenants[*tenantItr].empty = false; + self->createdTenants[tenantItr->first].empty = false; state Transaction checkTr(cx); loop { @@ -222,7 +250,7 @@ struct TenantManagementWorkload : TestWorkload { checkTr.setOption(FDBTransactionOptions::RAW_ACCESS); Optional val = wait(checkTr.get(self->keyName.withPrefix(entry.get().prefix))); ASSERT(val.present()); - ASSERT(val.get() == *tenantItr); + ASSERT(val.get() == tenantItr->first); break; } catch (Error& e) { wait(checkTr.onError(e)); @@ -230,7 +258,7 @@ struct TenantManagementWorkload : TestWorkload { } } - wait(self->checkTenant(cx, self, *tenantItr, self->createdTenants[*tenantItr])); + wait(self->checkTenant(cx, self, tenantItr->first, self->createdTenants[tenantItr->first])); } return Void(); } catch (Error& e) { @@ -244,14 +272,14 @@ struct TenantManagementWorkload : TestWorkload { ASSERT(tenantsToCreate.size() == 1); TraceEvent(SevError, "CreateTenantFailure") .error(e) - .detail("TenantName", *tenantsToCreate.begin()); + .detail("TenantName", tenantsToCreate.begin()->first); } return Void(); } else { try { wait(tr->onError(e)); } catch (Error& e) { - for (auto tenant : tenantsToCreate) { + for (auto [tenant, _] : tenantsToCreate) { TraceEvent(SevError, "CreateTenantFailure").error(e).detail("TenantName", tenant); } return Void(); @@ -395,12 +423,12 @@ struct TenantManagementWorkload : TestWorkload { ACTOR Future checkTenant(Database cx, TenantManagementWorkload* self, TenantName tenant, - TenantState tenantState) { + TenantData tenantData) { state Transaction tr(cx, tenant); loop { try { state RangeResult result = wait(tr.getRange(KeyRangeRef(""_sr, "\xff"_sr), 2)); - if (tenantState.empty) { + if (tenantData.empty) { ASSERT(result.size() == 0); } else { ASSERT(result.size() == 1); @@ -416,6 +444,8 @@ struct TenantManagementWorkload : TestWorkload { return Void(); } + // Convert the JSON document returned by the special-key space when reading tenant metadata + // into a TenantMapEntry static TenantMapEntry jsonToTenantMapEntry(ValueRef tenantJson) { json_spirit::mValue jsonObject; json_spirit::read_string(tenantJson.toString(), jsonObject); @@ -423,11 +453,17 @@ struct TenantManagementWorkload : TestWorkload { int64_t id; std::string prefix; + std::string tenantGroupStr; jsonDoc.get("id", id); jsonDoc.get("prefix", prefix); + Optional tenantGroup; + if (jsonDoc.tryGet("tenant_group", tenantGroupStr)) { + tenantGroup = TenantGroupNameRef(tenantGroupStr); + } + Key prefixKey = KeyRef(prefix); - TenantMapEntry entry(id, prefixKey.substr(0, prefixKey.size() - 8)); + TenantMapEntry entry(id, prefixKey.substr(0, prefixKey.size() - 8), tenantGroup); ASSERT(entry.prefix == prefixKey); return entry; @@ -437,7 +473,7 @@ struct TenantManagementWorkload : TestWorkload { state TenantName tenant = self->chooseTenantName(true); auto itr = self->createdTenants.find(tenant); state bool alreadyExists = itr != self->createdTenants.end(); - state TenantState tenantState = itr->second; + state TenantData tenantData = alreadyExists ? itr->second : TenantData(); state OperationType operationType = TenantManagementWorkload::randomOperationType(); state Reference tr = makeReference(cx); @@ -460,8 +496,9 @@ struct TenantManagementWorkload : TestWorkload { entry = _entry; } ASSERT(alreadyExists); - ASSERT(entry.id == tenantState.id); - wait(self->checkTenant(cx, self, tenant, tenantState)); + ASSERT(entry.id == tenantData.id); + ASSERT(entry.tenantGroup == tenantData.tenantGroup); + wait(self->checkTenant(cx, self, tenant, tenantData)); return Void(); } catch (Error& e) { state bool retry = true; @@ -600,10 +637,10 @@ struct TenantManagementWorkload : TestWorkload { ASSERT(newTenantEntry.present()); // Update Internal Tenant Map and check for correctness - TenantState tState = self->createdTenants[oldTenantName]; - self->createdTenants[newTenantName] = tState; + TenantData tData = self->createdTenants[oldTenantName]; + self->createdTenants[newTenantName] = tData; self->createdTenants.erase(oldTenantName); - if (!tState.empty) { + if (!tData.empty) { state Transaction insertTr(cx, newTenantName); loop { try { @@ -641,11 +678,94 @@ struct TenantManagementWorkload : TestWorkload { } } + ACTOR Future configureTenant(Database cx, TenantManagementWorkload* self) { + state TenantName tenant = self->chooseTenantName(true); + auto itr = self->createdTenants.find(tenant); + state bool exists = itr != self->createdTenants.end(); + state Reference tr = makeReference(cx); + + state std::map, Optional> configuration; + state Optional newTenantGroup; + state bool hasInvalidOption = deterministicRandom()->random01() < 0.1; + + if (!hasInvalidOption || deterministicRandom()->coinflip()) { + newTenantGroup = self->chooseTenantGroup(); + configuration["tenant_group"_sr] = newTenantGroup; + } + if (hasInvalidOption) { + configuration["invalid_option"_sr] = ""_sr; + hasInvalidOption = true; + } + + state bool hasInvalidSpecialKeyTuple = deterministicRandom()->random01() < 0.05; + + loop { + try { + tr->setOption(FDBTransactionOptions::SPECIAL_KEY_SPACE_ENABLE_WRITES); + for (auto [config, value] : configuration) { + Tuple t; + if (hasInvalidSpecialKeyTuple) { + // Wrong number of items + if (deterministicRandom()->coinflip()) { + int numItems = deterministicRandom()->randomInt(0, 3); + if (numItems > 0) { + t.append(tenant); + } + if (numItems > 1) { + t.append(config).append(""_sr); + } + } + // Wrong data types + else { + if (deterministicRandom()->coinflip()) { + t.append(0).append(config); + } else { + t.append(tenant).append(0); + } + } + } else { + t.append(tenant).append(config); + } + if (value.present()) { + tr->set(self->specialKeysTenantConfigPrefix.withSuffix(t.pack()), value.get()); + } else { + tr->clear(self->specialKeysTenantConfigPrefix.withSuffix(t.pack())); + } + } + + wait(tr->commit()); + + ASSERT(exists); + ASSERT(!hasInvalidOption); + ASSERT(!hasInvalidSpecialKeyTuple); + + self->createdTenants[tenant].tenantGroup = newTenantGroup; + return Void(); + } catch (Error& e) { + state Error error = e; + if (e.code() == error_code_tenant_not_found) { + ASSERT(!exists); + return Void(); + } else if (e.code() == error_code_special_keys_api_failure) { + ASSERT(hasInvalidSpecialKeyTuple || hasInvalidOption); + return Void(); + } + + try { + wait(tr->onError(e)); + } catch (Error&) { + TraceEvent(SevError, "ConfigureTenantFailure").error(error).detail("TenantName", tenant); + return Void(); + } + } + } + } + Future start(Database const& cx) override { return _start(cx, this); } ACTOR Future _start(Database cx, TenantManagementWorkload* self) { state double start = now(); while (now() < start + self->testDuration) { - state int operation = deterministicRandom()->randomInt(0, 5); + state int operation = deterministicRandom()->randomInt(0, 6); if (operation == 0) { wait(self->createTenant(cx, self)); } else if (operation == 1) { @@ -654,8 +774,10 @@ struct TenantManagementWorkload : TestWorkload { wait(self->getTenant(cx, self)); } else if (operation == 3) { wait(self->listTenants(cx, self)); - } else { + } else if (operation == 4) { wait(self->renameTenant(cx, self)); + } else if (operation == 5) { + wait(self->configureTenant(cx, self)); } } @@ -677,7 +799,7 @@ struct TenantManagementWorkload : TestWorkload { } } - state std::map::iterator itr = self->createdTenants.begin(); + state std::map::iterator itr = self->createdTenants.begin(); state std::vector> checkTenants; state TenantName beginTenant = ""_sr.withPrefix(self->localTenantNamePrefix); state TenantName endTenant = "\xff\xff"_sr.withPrefix(self->localTenantNamePrefix); @@ -690,6 +812,7 @@ struct TenantManagementWorkload : TestWorkload { for (auto tenant : tenants) { ASSERT(itr != self->createdTenants.end()); ASSERT(tenant.first == itr->first); + ASSERT(tenant.second.tenantGroup == itr->second.tenantGroup); checkTenants.push_back(self->checkTenant(cx, self, tenant.first, itr->second)); lastTenant = tenant.first; ++itr; diff --git a/flow/include/flow/Arena.h b/flow/include/flow/Arena.h index bfef0b4a6b..95ad264516 100644 --- a/flow/include/flow/Arena.h +++ b/flow/include/flow/Arena.h @@ -587,14 +587,20 @@ public: } // Removes bytes from begin up to and including the sep string, returns StringRef of the part before sep - StringRef eat(StringRef sep) { + StringRef eat(StringRef sep, bool* foundSep = nullptr) { for (int i = 0, iend = size() - sep.size(); i <= iend; ++i) { if (sep.compare(substr(i, sep.size())) == 0) { + if (foundSep) { + *foundSep = true; + } StringRef token = substr(0, i); *this = substr(i + sep.size()); return token; } } + if (foundSep) { + *foundSep = false; + } return eat(); } StringRef eat() { @@ -602,7 +608,9 @@ public: *this = StringRef(); return r; } - StringRef eat(const char* sep) { return eat(StringRef((const uint8_t*)sep, (int)strlen(sep))); } + StringRef eat(const char* sep, bool* foundSep = nullptr) { + return eat(StringRef((const uint8_t*)sep, (int)strlen(sep)), foundSep); + } // Return StringRef of bytes from begin() up to but not including the first byte matching any byte in sep, // and remove that sequence (including the sep byte) from *this // Returns and removes all bytes from *this if no bytes within sep were found diff --git a/flow/include/flow/ProtocolVersion.h b/flow/include/flow/ProtocolVersion.h index 323dcffcf0..811b8251e8 100644 --- a/flow/include/flow/ProtocolVersion.h +++ b/flow/include/flow/ProtocolVersion.h @@ -174,6 +174,7 @@ public: // introduced features PROTOCOL_VERSION_FEATURE(0x0FDB00B072000000LL, SWVersionTracking); PROTOCOL_VERSION_FEATURE(0x0FDB00B072000000LL, EncryptionAtRest); PROTOCOL_VERSION_FEATURE(0x0FDB00B072000000LL, ShardEncodeLocationMetaData); + PROTOCOL_VERSION_FEATURE(0x0FDB00B072000000LL, TenantGroups); }; template <> diff --git a/flow/include/flow/error_definitions.h b/flow/include/flow/error_definitions.h index 519a0cb8b0..ceb881bca9 100755 --- a/flow/include/flow/error_definitions.h +++ b/flow/include/flow/error_definitions.h @@ -228,11 +228,12 @@ ERROR( tenant_name_required, 2130, "Tenant name must be specified to access data ERROR( tenant_not_found, 2131, "Tenant does not exist" ) ERROR( tenant_already_exists, 2132, "A tenant with the given name already exists" ) ERROR( tenant_not_empty, 2133, "Cannot delete a non-empty tenant" ) -ERROR( invalid_tenant_name, 2134, "Tenant name cannot begin with \\xff"); -ERROR( tenant_prefix_allocator_conflict, 2135, "The database already has keys stored at the prefix allocated for the tenant"); -ERROR( tenants_disabled, 2136, "Tenants have been disabled in the cluster"); -ERROR( unknown_tenant, 2137, "Tenant is not available from this server") -ERROR( illegal_tenant_access, 2138, "Illegal tenant access") +ERROR( invalid_tenant_name, 2134, "Tenant name cannot begin with \\xff" ) +ERROR( tenant_prefix_allocator_conflict, 2135, "The database already has keys stored at the prefix allocated for the tenant" ) +ERROR( tenants_disabled, 2136, "Tenants have been disabled in the cluster" ) +ERROR( unknown_tenant, 2137, "Tenant is not available from this server" ) +ERROR( illegal_tenant_access, 2138, "Illegal tenant access" ) +ERROR( invalid_tenant_group_name, 2139, "Tenant group name cannot begin with \\xff" ) // 2200 - errors from bindings and official APIs ERROR( api_version_unset, 2200, "API version is not set" ) diff --git a/tests/slow/SwizzledTenantManagement.toml b/tests/slow/SwizzledTenantManagement.toml index e7129f1061..d34544eaa8 100644 --- a/tests/slow/SwizzledTenantManagement.toml +++ b/tests/slow/SwizzledTenantManagement.toml @@ -10,7 +10,7 @@ runSetup = true [[test.workload]] testName = 'TenantManagement' - maxTenants = 1000 + maxTenants = 1000 testDuration = 60 [[test.workload]] diff --git a/tests/slow/TenantManagement.toml b/tests/slow/TenantManagement.toml index 5848bdf4e3..f03bc421f2 100644 --- a/tests/slow/TenantManagement.toml +++ b/tests/slow/TenantManagement.toml @@ -10,5 +10,5 @@ runSetup = true [[test.workload]] testName = 'TenantManagement' - maxTenants = 1000 - testDuration = 60 + maxTenants = 1000 + testDuration = 60 From 7f3b51f31a923c60dca87838e14f70e032ff6cdb Mon Sep 17 00:00:00 2001 From: Jon Fu Date: Wed, 13 Jul 2022 15:34:27 -0700 Subject: [PATCH 02/20] initial commit to introduce tenant renaming to special keys --- fdbcli/TenantCommands.actor.cpp | 73 ++++++++++++++++++- fdbclient/TenantSpecialKeys.cpp | 6 ++ .../fdbclient/TenantManagement.actor.h | 15 +++- .../fdbclient/TenantSpecialKeys.actor.h | 42 +++++++++++ 4 files changed, 133 insertions(+), 3 deletions(-) diff --git a/fdbcli/TenantCommands.actor.cpp b/fdbcli/TenantCommands.actor.cpp index fa47b80f4b..1c2d35f81f 100644 --- a/fdbcli/TenantCommands.actor.cpp +++ b/fdbcli/TenantCommands.actor.cpp @@ -39,6 +39,8 @@ const KeyRangeRef tenantMapSpecialKeyRange(LiteralStringRef("\xff\xff/management LiteralStringRef("\xff\xff/management/tenant/map0")); const KeyRangeRef tenantConfigSpecialKeyRange(LiteralStringRef("\xff\xff/management/tenant/configure/"), LiteralStringRef("\xff\xff/management/tenant/configure0")); +const KeyRangeRef tenantRenameSpecialKeyRange(LiteralStringRef("\xff\xff/management/tenant/rename/"), + LiteralStringRef("\xff\xff/management/tenant/rename0")); Optional, Optional>> parseTenantConfiguration(std::vector const& tokens, int startIndex, bool allowUnset) { @@ -400,13 +402,82 @@ CommandFactory configureTenantFactory( "Updates the configuration for a tenant. Use `tenant_group=' to change the tenant group " "that a tenant is assigned to or `unset tenant_group' to remove a tenant from its tenant group.")); +// Helper function to extract tenant ID from json metadata string +int64_t getTenantId(Value metadata) { + json_spirit::mValue jsonObject; + json_spirit::read_string(metadata.toString(), jsonObject); + JSONDoc doc(jsonObject); + int64_t id; + doc.get("id", id); + return id; +} + // renametenant command ACTOR Future renameTenantCommandActor(Reference db, std::vector tokens) { if (tokens.size() != 3) { printUsage(tokens[0]); return false; } - wait(safeThreadFutureToFuture(TenantAPI::renameTenant(db, tokens[1], tokens[2]))); + state Reference tr = db->createTransaction(); + state Key tenantRenameKey = tenantRenameSpecialKeyRange.begin.withSuffix(tokens[1]); + state Key tenantOldNameKey = tenantMapSpecialKeyRange.begin.withSuffix(tokens[1]); + state Key tenantNewNameKey = tenantMapSpecialKeyRange.begin.withSuffix(tokens[2]); + state bool firstTry = true; + state int64_t id; + loop { + tr->setOption(FDBTransactionOptions::SPECIAL_KEY_SPACE_ENABLE_WRITES); + try { + // Hold the reference to the standalone's memory + state ThreadFuture> oldEntryFuture = tr->get(tenantOldNameKey); + state ThreadFuture> newEntryFuture = tr->get(tenantNewNameKey); + state Optional oldEntry = wait(safeThreadFutureToFuture(oldEntryFuture)); + state Optional newEntry = wait(safeThreadFutureToFuture(newEntryFuture)); + 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 = getTenantId(oldEntry.get()); + + firstTry = false; + } else { + // If we got commit_unknown_result, the rename may have already occurred. + if (newEntry.present()) { + int64_t checkId = getTenantId(newEntry.get()); + if (id == checkId) { + ASSERT(!oldEntry.present() || getTenantId(oldEntry.get()) != id); + return true; + } + // 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 = getTenantId(oldEntry.get()); + // 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) { + throw tenant_not_found(); + } + } + tr->set(tenantRenameKey, tokens[2]); + wait(safeThreadFutureToFuture(tr->commit())); + break; + } catch (Error& e) { + state Error err(e); + if (e.code() == error_code_special_keys_api_failure) { + std::string errorMsgStr = wait(getSpecialKeysFailureErrorMessage(tr)); + fmt::print(stderr, "ERROR: {}\n", errorMsgStr.c_str()); + return false; + } + wait(safeThreadFutureToFuture(tr->onError(err))); + } + } fmt::print( "The tenant `{}' has been renamed to `{}'\n", printable(tokens[1]).c_str(), printable(tokens[2]).c_str()); diff --git a/fdbclient/TenantSpecialKeys.cpp b/fdbclient/TenantSpecialKeys.cpp index 5570816684..8c60c935ea 100644 --- a/fdbclient/TenantSpecialKeys.cpp +++ b/fdbclient/TenantSpecialKeys.cpp @@ -26,12 +26,18 @@ const KeyRangeRef TenantRangeImpl::submoduleRange = KeyRangeRef("tenant/"_ template <> const KeyRangeRef TenantRangeImpl::mapSubRange = KeyRangeRef("map/"_sr, "map0"_sr); +template <> +const KeyRangeRef TenantRangeImpl::renameSubRange = KeyRangeRef("rename/"_sr, "rename0"_sr); + template <> const KeyRangeRef TenantRangeImpl::submoduleRange = KeyRangeRef(""_sr, "\xff"_sr); template <> const KeyRangeRef TenantRangeImpl::mapSubRange = KeyRangeRef("tenant_map/"_sr, "tenant_map0"_sr); +template <> +const KeyRangeRef TenantRangeImpl::renameSubRange = KeyRangeRef("tenant_rename/"_sr, "tenant_rename0"_sr); + template <> bool TenantRangeImpl::subRangeIntersects(KeyRangeRef subRange, KeyRangeRef range) { return subRange.intersects(range); diff --git a/fdbclient/include/fdbclient/TenantManagement.actor.h b/fdbclient/include/fdbclient/TenantManagement.actor.h index cd5f753fbb..4eab0a6380 100644 --- a/fdbclient/include/fdbclient/TenantManagement.actor.h +++ b/fdbclient/include/fdbclient/TenantManagement.actor.h @@ -301,6 +301,18 @@ Future> listTenants(Reference db, } } +ACTOR template +Future renameTenantTransaction(Transaction tr, TenantNameRef oldName, TenantNameRef newName) { + state Key oldNameKey = oldName.withPrefix(tenantMapPrefix); + state Key newNameKey = newName.withPrefix(tenantMapPrefix); + tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); + state Optional oldEntry = wait(tryGetTenantTransaction(tr, oldName)); + tr->clear(oldNameKey); + tr->set(newNameKey, oldEntry.get().encode()); + + return Void(); +} + ACTOR template Future renameTenant(Reference db, TenantName oldName, TenantName newName) { state Reference tr = db->createTransaction(); @@ -349,8 +361,7 @@ Future renameTenant(Reference db, TenantName oldName, TenantName newNa throw tenant_not_found(); } } - tr->clear(oldNameKey); - tr->set(newNameKey, oldEntry.get().encode()); + wait(renameTenantTransaction(tr, oldName, newName)); wait(safeThreadFutureToFuture(tr->commit())); TraceEvent("RenameTenantSuccess").detail("OldName", oldName).detail("NewName", newName); return Void(); diff --git a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h index b61b3a28b3..0e4f4eaa1f 100644 --- a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h +++ b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h @@ -207,10 +207,18 @@ private: return Void(); } + ACTOR static Future renameTenant(ReadYourWritesTransaction* ryw, + TenantNameRef oldName, + TenantNameRef newName) { + wait(TenantAPI::renameTenantTransaction(&ryw->getTransaction(), oldName, newName)); + return Void(); + } + public: // These ranges vary based on the template parameter const static KeyRangeRef submoduleRange; const static KeyRangeRef mapSubRange; + const static KeyRangeRef renameSubRange; // These sub-ranges should only be used if HasSubRanges=true const inline static KeyRangeRef configureSubRange = KeyRangeRef("configure/"_sr, "configure0"_sr); @@ -229,6 +237,8 @@ public: std::vector>> mapMutations; std::map, Optional>>> configMutations; + std::set renameSet; + std::vector> renameMutations; for (auto range : ranges) { if (!range.value().first) { @@ -259,12 +269,35 @@ public: false, "configure tenant", "invalid tenant configuration key")); throw special_keys_api_failure(); } + } else if (subRangeIntersects(renameSubRange, adjustedRange)) { + try { + StringRef oldName = adjustedRange.begin.removePrefix(renameSubRange.begin); + StringRef newName = range.value().second.get(); + // Do not allow overlapping renames in the same commit + // e.g. A->B, B->C + if (renameSet.count(oldName) || renameSet.count(newName)) { + throw tenant_already_exists(); + } + renameSet.insert(oldName); + renameSet.insert(newName); + renameMutations.push_back(std::make_pair(oldName, newName)); + } catch (Error& e) { + ryw->setSpecialKeySpaceErrorMsg( + ManagementAPIError::toJsonString(false, "rename tenant", "tenant rename conflict")); + throw special_keys_api_failure(); + } } } std::map, Optional>>>> tenantsToCreate; for (auto mapMutation : mapMutations) { TenantNameRef tenantName = mapMutation.first.begin; + auto set_iter = renameSet.lower_bound(tenantName); + if (set_iter != renameSet.end() && mapMutation.first.contains(*set_iter)) { + ryw->setSpecialKeySpaceErrorMsg( + ManagementAPIError::toJsonString(false, "rename tenant", "tenant rename conflict")); + throw special_keys_api_failure(); + } if (mapMutation.second.present()) { Optional, Optional>>> createMutations; auto itr = configMutations.find(tenantName); @@ -295,9 +328,18 @@ public: tenantManagementFutures.push_back(createTenants(ryw, tenantsToCreate)); } for (auto configMutation : configMutations) { + if (renameSet.count(configMutation.first)) { + ryw->setSpecialKeySpaceErrorMsg( + ManagementAPIError::toJsonString(false, "rename tenant", "tenant rename conflict")); + throw special_keys_api_failure(); + } tenantManagementFutures.push_back(changeTenantConfig(ryw, configMutation.first, configMutation.second)); } + for (auto renameMutation : renameMutations) { + tenantManagementFutures.push_back(renameTenant(ryw, renameMutation.first, renameMutation.second)); + } + return tag(waitForAll(tenantManagementFutures), Optional()); } }; From 59514597fda91572a4a50a6fe378a0e037f43409 Mon Sep 17 00:00:00 2001 From: Jon Fu Date: Wed, 13 Jul 2022 16:52:33 -0700 Subject: [PATCH 03/20] address code review comments --- fdbclient/TenantSpecialKeys.cpp | 6 ------ fdbclient/include/fdbclient/TenantManagement.actor.h | 2 +- .../include/fdbclient/TenantSpecialKeys.actor.h | 12 +++--------- 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/fdbclient/TenantSpecialKeys.cpp b/fdbclient/TenantSpecialKeys.cpp index 8c60c935ea..5570816684 100644 --- a/fdbclient/TenantSpecialKeys.cpp +++ b/fdbclient/TenantSpecialKeys.cpp @@ -26,18 +26,12 @@ const KeyRangeRef TenantRangeImpl::submoduleRange = KeyRangeRef("tenant/"_ template <> const KeyRangeRef TenantRangeImpl::mapSubRange = KeyRangeRef("map/"_sr, "map0"_sr); -template <> -const KeyRangeRef TenantRangeImpl::renameSubRange = KeyRangeRef("rename/"_sr, "rename0"_sr); - template <> const KeyRangeRef TenantRangeImpl::submoduleRange = KeyRangeRef(""_sr, "\xff"_sr); template <> const KeyRangeRef TenantRangeImpl::mapSubRange = KeyRangeRef("tenant_map/"_sr, "tenant_map0"_sr); -template <> -const KeyRangeRef TenantRangeImpl::renameSubRange = KeyRangeRef("tenant_rename/"_sr, "tenant_rename0"_sr); - template <> bool TenantRangeImpl::subRangeIntersects(KeyRangeRef subRange, KeyRangeRef range) { return subRange.intersects(range); diff --git a/fdbclient/include/fdbclient/TenantManagement.actor.h b/fdbclient/include/fdbclient/TenantManagement.actor.h index 4eab0a6380..45a9d65708 100644 --- a/fdbclient/include/fdbclient/TenantManagement.actor.h +++ b/fdbclient/include/fdbclient/TenantManagement.actor.h @@ -305,7 +305,7 @@ ACTOR template Future renameTenantTransaction(Transaction tr, TenantNameRef oldName, TenantNameRef newName) { state Key oldNameKey = oldName.withPrefix(tenantMapPrefix); state Key newNameKey = newName.withPrefix(tenantMapPrefix); - tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); + tr->setOption(FDBTransactionOptions::RAW_ACCESS); state Optional oldEntry = wait(tryGetTenantTransaction(tr, oldName)); tr->clear(oldNameKey); tr->set(newNameKey, oldEntry.get().encode()); diff --git a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h index 0e4f4eaa1f..5c98e31263 100644 --- a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h +++ b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h @@ -207,21 +207,14 @@ private: return Void(); } - ACTOR static Future renameTenant(ReadYourWritesTransaction* ryw, - TenantNameRef oldName, - TenantNameRef newName) { - wait(TenantAPI::renameTenantTransaction(&ryw->getTransaction(), oldName, newName)); - return Void(); - } - public: // These ranges vary based on the template parameter const static KeyRangeRef submoduleRange; const static KeyRangeRef mapSubRange; - const static KeyRangeRef renameSubRange; // These sub-ranges should only be used if HasSubRanges=true const inline static KeyRangeRef configureSubRange = KeyRangeRef("configure/"_sr, "configure0"_sr); + const inline static KeyRangeRef renameSubRange = KeyRangeRef("rename/"_sr, "rename0"_sr); explicit TenantRangeImpl(KeyRangeRef kr) : SpecialKeyRangeRWImpl(kr) {} @@ -337,7 +330,8 @@ public: } for (auto renameMutation : renameMutations) { - tenantManagementFutures.push_back(renameTenant(ryw, renameMutation.first, renameMutation.second)); + tenantManagementFutures.push_back(TenantAPI::renameTenantTransaction( + &ryw->getTransaction(), renameMutation.first, renameMutation.second)); } return tag(waitForAll(tenantManagementFutures), Optional()); From e53e9ca2f4b608b2405512b606155b32a2606923 Mon Sep 17 00:00:00 2001 From: Jon Fu Date: Wed, 13 Jul 2022 16:57:41 -0700 Subject: [PATCH 04/20] throw error directly --- .../fdbclient/TenantSpecialKeys.actor.h | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h index 5c98e31263..ff36df0114 100644 --- a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h +++ b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h @@ -263,22 +263,18 @@ public: throw special_keys_api_failure(); } } else if (subRangeIntersects(renameSubRange, adjustedRange)) { - try { - StringRef oldName = adjustedRange.begin.removePrefix(renameSubRange.begin); - StringRef newName = range.value().second.get(); - // Do not allow overlapping renames in the same commit - // e.g. A->B, B->C - if (renameSet.count(oldName) || renameSet.count(newName)) { - throw tenant_already_exists(); - } - renameSet.insert(oldName); - renameSet.insert(newName); - renameMutations.push_back(std::make_pair(oldName, newName)); - } catch (Error& e) { + StringRef oldName = adjustedRange.begin.removePrefix(renameSubRange.begin); + StringRef newName = range.value().second.get(); + // Do not allow overlapping renames in the same commit + // e.g. A->B, B->C + if (renameSet.count(oldName) || renameSet.count(newName)) { ryw->setSpecialKeySpaceErrorMsg( ManagementAPIError::toJsonString(false, "rename tenant", "tenant rename conflict")); throw special_keys_api_failure(); } + renameSet.insert(oldName); + renameSet.insert(newName); + renameMutations.push_back(std::make_pair(oldName, newName)); } } From 8a31fa87a838b0c843e10e490bf5b5878c394cec Mon Sep 17 00:00:00 2001 From: Jon Fu Date: Fri, 15 Jul 2022 16:22:37 -0700 Subject: [PATCH 05/20] add other operations to test workload --- .../fdbclient/TenantManagement.actor.h | 11 +- .../TenantManagementWorkload.actor.cpp | 131 ++++++++++++------ 2 files changed, 97 insertions(+), 45 deletions(-) diff --git a/fdbclient/include/fdbclient/TenantManagement.actor.h b/fdbclient/include/fdbclient/TenantManagement.actor.h index 45a9d65708..3535221759 100644 --- a/fdbclient/include/fdbclient/TenantManagement.actor.h +++ b/fdbclient/include/fdbclient/TenantManagement.actor.h @@ -306,7 +306,16 @@ Future renameTenantTransaction(Transaction tr, TenantNameRef oldName, Tena state Key oldNameKey = oldName.withPrefix(tenantMapPrefix); state Key newNameKey = newName.withPrefix(tenantMapPrefix); tr->setOption(FDBTransactionOptions::RAW_ACCESS); - state Optional oldEntry = wait(tryGetTenantTransaction(tr, oldName)); + state Optional oldEntry; + state Optional newEntry; + wait(store(oldEntry, tryGetTenantTransaction(tr, oldName)) && + store(newEntry, tryGetTenantTransaction(tr, newName))); + if (!oldEntry.present()) { + throw tenant_not_found(); + } + if (newEntry.present()) { + throw tenant_already_exists(); + } tr->clear(oldNameKey); tr->set(newNameKey, oldEntry.get().encode()); diff --git a/fdbserver/workloads/TenantManagementWorkload.actor.cpp b/fdbserver/workloads/TenantManagementWorkload.actor.cpp index 14422afa16..1613356132 100644 --- a/fdbserver/workloads/TenantManagementWorkload.actor.cpp +++ b/fdbserver/workloads/TenantManagementWorkload.actor.cpp @@ -58,6 +58,9 @@ struct TenantManagementWorkload : TestWorkload { const Key specialKeysTenantConfigPrefix = SpecialKeySpace::getModuleRange(SpecialKeySpace::MODULE::MANAGEMENT) .begin.withSuffix(TenantRangeImpl::submoduleRange.begin) .withSuffix(TenantRangeImpl::configureSubRange.begin); + const Key specialKeysTenantRenamePrefix = SpecialKeySpace::getModuleRange(SpecialKeySpace::MODULE::MANAGEMENT) + .begin.withSuffix(TenantRangeImpl::submoduleRange.begin) + .withSuffix(TenantRangeImpl::renameSubRange.begin); int maxTenants; int maxTenantGroups; @@ -597,20 +600,56 @@ struct TenantManagementWorkload : TestWorkload { } } + // Helper function that checks tenant keyspace and updates internal Tenant Map after a rename operation + ACTOR Future verifyTenantRename(Database cx, + TenantManagementWorkload* self, + TenantName oldTenantName, + TenantName newTenantName) { + state Optional oldTenantEntry = wait(TenantAPI::tryGetTenant(cx.getReference(), oldTenantName)); + state Optional newTenantEntry = wait(TenantAPI::tryGetTenant(cx.getReference(), newTenantName)); + ASSERT(!oldTenantEntry.present()); + ASSERT(newTenantEntry.present()); + TenantData tData = self->createdTenants[oldTenantName]; + self->createdTenants[newTenantName] = tData; + self->createdTenants.erase(oldTenantName); + if (!tData.empty) { + state Transaction insertTr(cx, newTenantName); + loop { + try { + insertTr.set(self->keyName, newTenantName); + wait(insertTr.commit()); + break; + } catch (Error& e) { + wait(insertTr.onError(e)); + } + } + } + return Void(); + } + ACTOR Future renameTenant(Database cx, TenantManagementWorkload* self) { - // Currently only supporting MANAGEMENT_DATABASE op, so numTenants should always be 1 - // state OperationType operationType = TenantManagementWorkload::randomOperationType(); - int numTenants = 1; + state OperationType operationType = TenantManagementWorkload::randomOperationType(); + state int numTenants = 1; + state Reference tr = makeReference(cx); + + if (operationType == OperationType::SPECIAL_KEYS || operationType == OperationType::MANAGEMENT_TRANSACTION) { + numTenants = deterministicRandom()->randomInt(1, 5); + } state std::vector oldTenantNames; state std::vector newTenantNames; + state std::set allTenantNames; state bool tenantExists = false; state bool tenantNotFound = false; + state bool tenantOverlap = false; for (int i = 0; i < numTenants; ++i) { TenantName oldTenant = self->chooseTenantName(false); TenantName newTenant = self->chooseTenantName(false); - newTenantNames.push_back(newTenant); + tenantOverlap = tenantOverlap || allTenantNames.count(oldTenant) || allTenantNames.count(newTenant); oldTenantNames.push_back(oldTenant); + newTenantNames.push_back(newTenant); + allTenantNames.insert(oldTenant); + allTenantNames.insert(newTenant); if (!self->createdTenants.count(oldTenant)) { tenantNotFound = true; } @@ -621,59 +660,63 @@ struct TenantManagementWorkload : TestWorkload { loop { try { - ASSERT(oldTenantNames.size() == 1); - state int tenantIndex = 0; - for (; tenantIndex != oldTenantNames.size(); ++tenantIndex) { - state TenantName oldTenantName = oldTenantNames[tenantIndex]; - state TenantName newTenantName = newTenantNames[tenantIndex]; - // Perform rename, then check against the DB for the new results - wait(TenantAPI::renameTenant(cx.getReference(), oldTenantName, newTenantName)); - ASSERT(!tenantNotFound && !tenantExists); - state Optional oldTenantEntry = - wait(TenantAPI::tryGetTenant(cx.getReference(), oldTenantName)); - state Optional newTenantEntry = - wait(TenantAPI::tryGetTenant(cx.getReference(), newTenantName)); - ASSERT(!oldTenantEntry.present()); - ASSERT(newTenantEntry.present()); - - // Update Internal Tenant Map and check for correctness - TenantData tData = self->createdTenants[oldTenantName]; - self->createdTenants[newTenantName] = tData; - self->createdTenants.erase(oldTenantName); - if (!tData.empty) { - state Transaction insertTr(cx, newTenantName); - loop { - try { - insertTr.set(self->keyName, newTenantName); - wait(insertTr.commit()); - break; - } catch (Error& e) { - wait(insertTr.onError(e)); - } - } + if (operationType == OperationType::SPECIAL_KEYS) { + tr->setOption(FDBTransactionOptions::SPECIAL_KEY_SPACE_ENABLE_WRITES); + for (int i = 0; i != numTenants; ++i) { + tr->set(self->specialKeysTenantRenamePrefix.withSuffix(oldTenantNames[i]), newTenantNames[i]); } + wait(tr->commit()); + ASSERT(!tenantNotFound && !tenantExists && !tenantOverlap); + } else if (operationType == OperationType::MANAGEMENT_DATABASE) { + ASSERT(oldTenantNames.size() == 1 && newTenantNames.size() == 1); + wait(TenantAPI::renameTenant(cx.getReference(), oldTenantNames[0], newTenantNames[0])); + // Perform rename, then check against the DB for the new results + ASSERT(!tenantNotFound && !tenantExists); + } else { // operationType == OperationType::MANAGEMENT_TRANSACTION + tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); + std::vector> renameFutures; + for (int i = 0; i != numTenants; ++i) { + renameFutures.push_back( + success(TenantAPI::renameTenantTransaction(tr, oldTenantNames[i], newTenantNames[i]))); + } + wait(waitForAll(renameFutures)); + wait(tr->commit()); + ASSERT(!tenantNotFound && !tenantExists); + } + state int tIndex = 0; + for (; tIndex != numTenants; ++tIndex) { + wait(self->verifyTenantRename(cx, self, oldTenantNames[tIndex], newTenantNames[tIndex])); + state TenantName newTenantName = newTenantNames[tIndex]; wait(self->checkTenant(cx, self, newTenantName, self->createdTenants[newTenantName])); } return Void(); } catch (Error& e) { - ASSERT(oldTenantNames.size() == 1); + if (operationType == OperationType::MANAGEMENT_DATABASE) { + ASSERT(oldTenantNames.size() == 1 && newTenantNames.size() == 1); + } if (e.code() == error_code_tenant_not_found) { TraceEvent("RenameTenantOldTenantNotFound") - .detail("OldTenantName", oldTenantNames[0]) - .detail("NewTenantName", newTenantNames[0]); + .detail("OldTenantNames", describe(oldTenantNames)) + .detail("NewTenantNames", describe(newTenantNames)); ASSERT(tenantNotFound); + return Void(); } else if (e.code() == error_code_tenant_already_exists) { TraceEvent("RenameTenantNewTenantAlreadyExists") - .detail("OldTenantName", oldTenantNames[0]) - .detail("NewTenantName", newTenantNames[0]); + .detail("OldTenantNames", describe(oldTenantNames)) + .detail("NewTenantNames", describe(newTenantNames)); ASSERT(tenantExists); + return Void(); } else { - TraceEvent(SevError, "RenameTenantFailure") - .error(e) - .detail("OldTenantName", oldTenantNames[0]) - .detail("NewTenantName", newTenantNames[0]); + try { + wait(tr->onError(e)); + } catch (Error& e) { + TraceEvent(SevError, "RenameTenantFailure") + .error(e) + .detail("OldTenantNames", describe(oldTenantNames)) + .detail("NewTenantNames", describe(newTenantNames)); + return Void(); + } } - return Void(); } } } From 762c01d5676b80b2fe7d4778457a71b45d70abc8 Mon Sep 17 00:00:00 2001 From: Jon Fu Date: Wed, 20 Jul 2022 13:43:13 -0700 Subject: [PATCH 06/20] add error check for overlap --- fdbserver/workloads/TenantManagementWorkload.actor.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fdbserver/workloads/TenantManagementWorkload.actor.cpp b/fdbserver/workloads/TenantManagementWorkload.actor.cpp index 1613356132..76123b9ff7 100644 --- a/fdbserver/workloads/TenantManagementWorkload.actor.cpp +++ b/fdbserver/workloads/TenantManagementWorkload.actor.cpp @@ -706,6 +706,12 @@ struct TenantManagementWorkload : TestWorkload { .detail("NewTenantNames", describe(newTenantNames)); ASSERT(tenantExists); return Void(); + } else if (e.code() == error_code_special_keys_api_failure) { + TraceEvent("RenameTenantNameConflict") + .detail("OldTenantNames", describe(oldTenantNames)) + .detail("NewTenantNames", describe(newTenantNames)); + ASSERT(tenantOverlap); + return Void(); } else { try { wait(tr->onError(e)); From 818a9fb057edcb157ae7cbd8b0193fd5c8c42282 Mon Sep 17 00:00:00 2001 From: Jon Fu Date: Fri, 22 Jul 2022 12:40:36 -0700 Subject: [PATCH 07/20] fix some test logic to account for some missing error handling in special keys and txn case --- .../fdbclient/TenantSpecialKeys.actor.h | 4 +- .../TenantManagementWorkload.actor.cpp | 89 ++++++++++++++++--- 2 files changed, 77 insertions(+), 16 deletions(-) diff --git a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h index ff36df0114..21e43f4546 100644 --- a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h +++ b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h @@ -266,8 +266,8 @@ public: StringRef oldName = adjustedRange.begin.removePrefix(renameSubRange.begin); StringRef newName = range.value().second.get(); // Do not allow overlapping renames in the same commit - // e.g. A->B, B->C - if (renameSet.count(oldName) || renameSet.count(newName)) { + // e.g. A->B + B->C, D->D + if (renameSet.count(oldName) || renameSet.count(newName) || oldName == newName) { ryw->setSpecialKeySpaceErrorMsg( ManagementAPIError::toJsonString(false, "rename tenant", "tenant rename conflict")); throw special_keys_api_failure(); diff --git a/fdbserver/workloads/TenantManagementWorkload.actor.cpp b/fdbserver/workloads/TenantManagementWorkload.actor.cpp index 76123b9ff7..b7e1443bf4 100644 --- a/fdbserver/workloads/TenantManagementWorkload.actor.cpp +++ b/fdbserver/workloads/TenantManagementWorkload.actor.cpp @@ -612,8 +612,8 @@ struct TenantManagementWorkload : TestWorkload { TenantData tData = self->createdTenants[oldTenantName]; self->createdTenants[newTenantName] = tData; self->createdTenants.erase(oldTenantName); + state Transaction insertTr(cx, newTenantName); if (!tData.empty) { - state Transaction insertTr(cx, newTenantName); loop { try { insertTr.set(self->keyName, newTenantName); @@ -623,6 +623,30 @@ struct TenantManagementWorkload : TestWorkload { wait(insertTr.onError(e)); } } + } else { + loop { + try { + insertTr.clear(KeyRangeRef(""_sr, "\xff"_sr)); + wait(insertTr.commit()); + break; + } catch (Error& e) { + wait(insertTr.onError(e)); + } + } + } + return Void(); + } + + ACTOR Future verifyTenantRenames(Database cx, + TenantManagementWorkload* self, + std::vector oldTenantNames, + std::vector newTenantNames, + int numTenants) { + state int tIndex = 0; + for (; tIndex != numTenants; ++tIndex) { + wait(self->verifyTenantRename(cx, self, oldTenantNames[tIndex], newTenantNames[tIndex])); + state TenantName newTenantName = newTenantNames[tIndex]; + wait(self->checkTenant(cx, self, newTenantName, self->createdTenants[newTenantName])); } return Void(); } @@ -639,13 +663,18 @@ struct TenantManagementWorkload : TestWorkload { state std::vector oldTenantNames; state std::vector newTenantNames; state std::set allTenantNames; + + // Tenant Error flags state bool tenantExists = false; state bool tenantNotFound = false; state bool tenantOverlap = false; + state bool unknownResult = false; + for (int i = 0; i < numTenants; ++i) { TenantName oldTenant = self->chooseTenantName(false); TenantName newTenant = self->chooseTenantName(false); - tenantOverlap = tenantOverlap || allTenantNames.count(oldTenant) || allTenantNames.count(newTenant); + tenantOverlap = tenantOverlap || oldTenant == newTenant || allTenantNames.count(oldTenant) || + allTenantNames.count(newTenant); oldTenantNames.push_back(oldTenant); newTenantNames.push_back(newTenant); allTenantNames.insert(oldTenant); @@ -662,20 +691,40 @@ struct TenantManagementWorkload : TestWorkload { try { if (operationType == OperationType::SPECIAL_KEYS) { tr->setOption(FDBTransactionOptions::SPECIAL_KEY_SPACE_ENABLE_WRITES); + // Some of the tenant error flags may be true, but it is possible for the commit not to + // throw an error. This is because if we get A->B + A->C + // the special keys uses RYW and only applies the latest op, while the violation + // may have been from one of the earlier ops, or overlapping on the same "old" key for (int i = 0; i != numTenants; ++i) { + if (std::count(oldTenantNames.begin(), oldTenantNames.end(), oldTenantNames[i]) > 1) { + // Throw instead of committing to avoid mismatch in internal map and actual DB state. + ASSERT(tenantOverlap); + throw special_keys_api_failure(); + } tr->set(self->specialKeysTenantRenamePrefix.withSuffix(oldTenantNames[i]), newTenantNames[i]); } wait(tr->commit()); - ASSERT(!tenantNotFound && !tenantExists && !tenantOverlap); } else if (operationType == OperationType::MANAGEMENT_DATABASE) { ASSERT(oldTenantNames.size() == 1 && newTenantNames.size() == 1); wait(TenantAPI::renameTenant(cx.getReference(), oldTenantNames[0], newTenantNames[0])); - // Perform rename, then check against the DB for the new results ASSERT(!tenantNotFound && !tenantExists); } else { // operationType == OperationType::MANAGEMENT_TRANSACTION tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); + // These types of overlap will also fail as tenantNotFound or tenantExists: + // A->A + // A->B + B->C std::vector> renameFutures; for (int i = 0; i != numTenants; ++i) { + // These types of overlap: + // A->B + A->C (Old Name Overlap) + // A->C + B->C (New Name Overlap) + // cannot be detected since everything happens in one commit + // and will not observe each other's changes in time + if (std::count(oldTenantNames.begin(), oldTenantNames.end(), oldTenantNames[i]) > 1 || + std::count(newTenantNames.begin(), newTenantNames.end(), newTenantNames[i]) > 1) { + ASSERT(tenantOverlap); + throw special_keys_api_failure(); + } renameFutures.push_back( success(TenantAPI::renameTenantTransaction(tr, oldTenantNames[i], newTenantNames[i]))); } @@ -683,12 +732,7 @@ struct TenantManagementWorkload : TestWorkload { wait(tr->commit()); ASSERT(!tenantNotFound && !tenantExists); } - state int tIndex = 0; - for (; tIndex != numTenants; ++tIndex) { - wait(self->verifyTenantRename(cx, self, oldTenantNames[tIndex], newTenantNames[tIndex])); - state TenantName newTenantName = newTenantNames[tIndex]; - wait(self->checkTenant(cx, self, newTenantName, self->createdTenants[newTenantName])); - } + wait(self->verifyTenantRenames(cx, self, oldTenantNames, newTenantNames, numTenants)); return Void(); } catch (Error& e) { if (operationType == OperationType::MANAGEMENT_DATABASE) { @@ -697,14 +741,24 @@ struct TenantManagementWorkload : TestWorkload { if (e.code() == error_code_tenant_not_found) { TraceEvent("RenameTenantOldTenantNotFound") .detail("OldTenantNames", describe(oldTenantNames)) - .detail("NewTenantNames", describe(newTenantNames)); - ASSERT(tenantNotFound); + .detail("NewTenantNames", describe(newTenantNames)) + .detail("CommitUnknownResult", unknownResult); + if (unknownResult) { + wait(self->verifyTenantRenames(cx, self, oldTenantNames, newTenantNames, numTenants)); + } else { + ASSERT(tenantNotFound); + } return Void(); } else if (e.code() == error_code_tenant_already_exists) { TraceEvent("RenameTenantNewTenantAlreadyExists") .detail("OldTenantNames", describe(oldTenantNames)) - .detail("NewTenantNames", describe(newTenantNames)); - ASSERT(tenantExists); + .detail("NewTenantNames", describe(newTenantNames)) + .detail("CommitUnknownResult", unknownResult); + if (unknownResult) { + wait(self->verifyTenantRenames(cx, self, oldTenantNames, newTenantNames, numTenants)); + } else { + ASSERT(tenantExists); + } return Void(); } else if (e.code() == error_code_special_keys_api_failure) { TraceEvent("RenameTenantNameConflict") @@ -714,6 +768,13 @@ struct TenantManagementWorkload : TestWorkload { return Void(); } else { try { + // In the case of commit_unknown_result, assume we continue retrying + // until it's successful. Next loop around may throw error because it's + // already been moved, so account for that and update internal map as needed. + if (e.code() == error_code_commit_unknown_result) { + TraceEvent("RenameTenantCommitUnknownResult").error(e); + unknownResult = true; + } wait(tr->onError(e)); } catch (Error& e) { TraceEvent(SevError, "RenameTenantFailure") From 2b6f349649c233ae9260ef9b17809072bb4bd568 Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Fri, 22 Jul 2022 15:37:48 -0700 Subject: [PATCH 08/20] Better API version handling for fdbcli tenant commands --- fdbcli/TenantCommands.actor.cpp | 45 +++++++++++++++++++--------- fdbcli/fdbcli.actor.cpp | 18 +++++++++-- fdbcli/include/fdbcli/fdbcli.actor.h | 6 ++-- 3 files changed, 49 insertions(+), 20 deletions(-) diff --git a/fdbcli/TenantCommands.actor.cpp b/fdbcli/TenantCommands.actor.cpp index cb93b6c210..c341b0a248 100644 --- a/fdbcli/TenantCommands.actor.cpp +++ b/fdbcli/TenantCommands.actor.cpp @@ -35,10 +35,21 @@ namespace fdb_cli { -const KeyRangeRef tenantMapSpecialKeyRange(LiteralStringRef("\xff\xff/management/tenant/map/"), - LiteralStringRef("\xff\xff/management/tenant/map0")); -const KeyRangeRef tenantConfigSpecialKeyRange(LiteralStringRef("\xff\xff/management/tenant/configure/"), - LiteralStringRef("\xff\xff/management/tenant/configure0")); +const KeyRangeRef tenantMapSpecialKeyRange720("\xff\xff/management/tenant/map/"_sr, + "\xff\xff/management/tenant/map0"_sr); +const KeyRangeRef tenantConfigSpecialKeyRange("\xff\xff/management/tenant/configure/"_sr, + "\xff\xff/management/tenant/configure0"_sr); + +const KeyRangeRef tenantMapSpecialKeyRange710("\xff\xff/management/tenant_map/"_sr, + "\xff\xff/management/tenant_map0"_sr); + +KeyRangeRef const& tenantMapSpecialKeyRange(int apiVersion) { + if (apiVersion >= 720) { + return tenantMapSpecialKeyRange720; + } else { + return tenantMapSpecialKeyRange710; + } +} Optional, Optional>> parseTenantConfiguration(std::vector const& tokens, int startIndex, bool allowUnset) { @@ -94,13 +105,13 @@ void applyConfiguration(Reference tr, } // createtenant command -ACTOR Future createTenantCommandActor(Reference db, std::vector tokens) { +ACTOR Future createTenantCommandActor(Reference db, std::vector tokens, int apiVersion) { if (tokens.size() < 2 || tokens.size() > 3) { printUsage(tokens[0]); return false; } - state Key tenantNameKey = tenantMapSpecialKeyRange.begin.withSuffix(tokens[1]); + state Key tenantNameKey = tenantMapSpecialKeyRange(apiVersion).begin.withSuffix(tokens[1]); state Reference tr = db->createTransaction(); state bool doneExistenceCheck = false; @@ -111,6 +122,11 @@ ACTOR Future createTenantCommandActor(Reference db, std::vector return false; } + if (apiVersion < 720 && !configuration.get().empty()) { + fmt::print(stderr, "ERROR: tenants do not accept configuration options before API version 720.\n"); + return false; + } + loop { tr->setOption(FDBTransactionOptions::SPECIAL_KEY_SPACE_ENABLE_WRITES); try { @@ -149,13 +165,13 @@ CommandFactory createTenantFactory("createtenant", "Creates a new tenant in the cluster with the specified name.")); // deletetenant command -ACTOR Future deleteTenantCommandActor(Reference db, std::vector tokens) { +ACTOR Future deleteTenantCommandActor(Reference db, std::vector tokens, int apiVersion) { if (tokens.size() != 2) { printUsage(tokens[0]); return false; } - state Key tenantNameKey = tenantMapSpecialKeyRange.begin.withSuffix(tokens[1]); + state Key tenantNameKey = tenantMapSpecialKeyRange(apiVersion).begin.withSuffix(tokens[1]); state Reference tr = db->createTransaction(); state bool doneExistenceCheck = false; @@ -198,7 +214,7 @@ CommandFactory deleteTenantFactory( "Deletes a tenant from the cluster. Deletion will be allowed only if the specified tenant contains no data.")); // listtenants command -ACTOR Future listTenantsCommandActor(Reference db, std::vector tokens) { +ACTOR Future listTenantsCommandActor(Reference db, std::vector tokens, int apiVersion) { if (tokens.size() > 4) { printUsage(tokens[0]); return false; @@ -226,8 +242,8 @@ ACTOR Future listTenantsCommandActor(Reference db, std::vector< } } - state Key beginTenantKey = tenantMapSpecialKeyRange.begin.withSuffix(beginTenant); - state Key endTenantKey = tenantMapSpecialKeyRange.begin.withSuffix(endTenant); + state Key beginTenantKey = tenantMapSpecialKeyRange(apiVersion).begin.withSuffix(beginTenant); + state Key endTenantKey = tenantMapSpecialKeyRange(apiVersion).begin.withSuffix(endTenant); state Reference tr = db->createTransaction(); loop { @@ -247,8 +263,9 @@ ACTOR Future listTenantsCommandActor(Reference db, std::vector< int index = 0; for (auto tenant : tenants) { - fmt::print( - " {}. {}\n", ++index, printable(tenant.key.removePrefix(tenantMapSpecialKeyRange.begin)).c_str()); + fmt::print(" {}. {}\n", + ++index, + printable(tenant.key.removePrefix(tenantMapSpecialKeyRange(apiVersion).begin)).c_str()); } return true; @@ -279,7 +296,7 @@ ACTOR Future getTenantCommandActor(Reference db, std::vector tr = db->createTransaction(); loop { diff --git a/fdbcli/fdbcli.actor.cpp b/fdbcli/fdbcli.actor.cpp index 26f4a8469c..d51adcb241 100644 --- a/fdbcli/fdbcli.actor.cpp +++ b/fdbcli/fdbcli.actor.cpp @@ -1909,14 +1909,14 @@ ACTOR Future cli(CLIOptions opt, LineNoise* plinenoise) { } if (tokencmp(tokens[0], "createtenant")) { - bool _result = wait(makeInterruptable(createTenantCommandActor(db, tokens))); + bool _result = wait(makeInterruptable(createTenantCommandActor(db, tokens, opt.apiVersion))); if (!_result) is_error = true; continue; } if (tokencmp(tokens[0], "deletetenant")) { - bool _result = wait(makeInterruptable(deleteTenantCommandActor(db, tokens))); + bool _result = wait(makeInterruptable(deleteTenantCommandActor(db, tokens, opt.apiVersion))); if (!_result) is_error = true; else if (tenantName.present() && tokens[1] == tenantName.get()) { @@ -1928,7 +1928,7 @@ ACTOR Future cli(CLIOptions opt, LineNoise* plinenoise) { } if (tokencmp(tokens[0], "listtenants")) { - bool _result = wait(makeInterruptable(listTenantsCommandActor(db, tokens))); + bool _result = wait(makeInterruptable(listTenantsCommandActor(db, tokens, opt.apiVersion))); if (!_result) is_error = true; continue; @@ -1942,6 +1942,12 @@ ACTOR Future cli(CLIOptions opt, LineNoise* plinenoise) { } if (tokencmp(tokens[0], "configuretenant")) { + if (opt.apiVersion < 720) { + fmt::print(stderr, "ERROR: tenants cannot be configured before API version 720.\n"); + is_error = true; + continue; + } + bool _result = wait(makeInterruptable(configureTenantCommandActor(db, tokens))); if (!_result) is_error = true; @@ -1949,6 +1955,12 @@ ACTOR Future cli(CLIOptions opt, LineNoise* plinenoise) { } if (tokencmp(tokens[0], "renametenant")) { + if (opt.apiVersion < 720) { + fmt::print(stderr, "ERROR: tenants cannot be renamed before API version 720.\n"); + is_error = true; + continue; + } + bool _result = wait(makeInterruptable(renameTenantCommandActor(db, tokens))); if (!_result) is_error = true; diff --git a/fdbcli/include/fdbcli/fdbcli.actor.h b/fdbcli/include/fdbcli/fdbcli.actor.h index 3b497b17d5..1ea649a805 100644 --- a/fdbcli/include/fdbcli/fdbcli.actor.h +++ b/fdbcli/include/fdbcli/fdbcli.actor.h @@ -166,11 +166,11 @@ ACTOR Future consistencyCheckCommandActor(Reference tr, // coordinators command ACTOR Future coordinatorsCommandActor(Reference db, std::vector tokens); // createtenant command -ACTOR Future createTenantCommandActor(Reference db, std::vector tokens); +ACTOR Future createTenantCommandActor(Reference db, std::vector tokens, int apiVersion); // datadistribution command ACTOR Future dataDistributionCommandActor(Reference db, std::vector tokens); // deletetenant command -ACTOR Future deleteTenantCommandActor(Reference db, std::vector tokens); +ACTOR Future deleteTenantCommandActor(Reference db, std::vector tokens, int apiVersion); // exclude command ACTOR Future excludeCommandActor(Reference db, std::vector tokens, Future warn); // expensive_data_check command @@ -196,7 +196,7 @@ ACTOR Future killCommandActor(Reference db, std::vector tokens, std::map>* address_interface); // listtenants command -ACTOR Future listTenantsCommandActor(Reference db, std::vector tokens); +ACTOR Future listTenantsCommandActor(Reference db, std::vector tokens, int apiVersion); // lock/unlock command ACTOR Future lockCommandActor(Reference db, std::vector tokens); ACTOR Future unlockDatabaseActor(Reference db, UID uid); From 1d3a1290705264fd78f01356b73ddd741bc4ecae Mon Sep 17 00:00:00 2001 From: Jon Fu Date: Fri, 22 Jul 2022 16:09:08 -0700 Subject: [PATCH 09/20] fix compile errors from merge --- fdbclient/TenantManagement.actor.cpp | 39 --------- fdbclient/include/fdbclient/Tenant.h | 2 +- .../fdbclient/TenantManagement.actor.h | 2 - .../fdbclient/TenantSpecialKeys.actor.h | 3 +- .../TenantManagementWorkload.actor.cpp | 85 +------------------ 5 files changed, 4 insertions(+), 127 deletions(-) delete mode 100644 fdbclient/TenantManagement.actor.cpp diff --git a/fdbclient/TenantManagement.actor.cpp b/fdbclient/TenantManagement.actor.cpp deleted file mode 100644 index 2d458be02c..0000000000 --- a/fdbclient/TenantManagement.actor.cpp +++ /dev/null @@ -1,39 +0,0 @@ -/* - * TenantManagement.actor.cpp - * - * This source file is part of the FoundationDB open source project - * - * Copyright 2013-2022 Apple Inc. and the FoundationDB project authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include -#include -#include "fdbclient/SystemData.h" -#include "fdbclient/TenantManagement.actor.h" -#include "fdbclient/Tuple.h" -#include "flow/actorcompiler.h" // has to be last include - -namespace TenantAPI { - -Key getTenantGroupIndexKey(TenantGroupNameRef tenantGroup, Optional tenant) { - Tuple tuple; - tuple.append(tenantGroup); - if (tenant.present()) { - tuple.append(tenant.get()); - } - return tenantGroupTenantIndexKeys.begin.withSuffix(tuple.pack()); -} - -} // namespace TenantAPI \ No newline at end of file diff --git a/fdbclient/include/fdbclient/Tenant.h b/fdbclient/include/fdbclient/Tenant.h index 4d014cf2d5..133c1496e7 100644 --- a/fdbclient/include/fdbclient/Tenant.h +++ b/fdbclient/include/fdbclient/Tenant.h @@ -61,7 +61,7 @@ public: bool matchesConfiguration(TenantMapEntry const& other) const; void configure(Standalone parameter, Optional value); - Value encode() const { return ObjectWriter::toValue(*this, IncludeVersion(ProtocolVersion::withTenantGroups())); } + Value encode() const { return ObjectWriter::toValue(*this, IncludeVersion(ProtocolVersion::withTenants())); } static TenantMapEntry decode(ValueRef const& value) { TenantMapEntry entry; diff --git a/fdbclient/include/fdbclient/TenantManagement.actor.h b/fdbclient/include/fdbclient/TenantManagement.actor.h index b4c38df2a0..081763c733 100644 --- a/fdbclient/include/fdbclient/TenantManagement.actor.h +++ b/fdbclient/include/fdbclient/TenantManagement.actor.h @@ -371,8 +371,6 @@ Future>> listTenants(Reference ACTOR template Future renameTenantTransaction(Transaction tr, TenantNameRef oldName, TenantNameRef newName) { - state Key oldNameKey = oldName.withPrefix(tenantMapPrefix); - state Key newNameKey = newName.withPrefix(tenantMapPrefix); tr->setOption(FDBTransactionOptions::RAW_ACCESS); state Optional oldEntry; state Optional newEntry; diff --git a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h index 08f4ceb027..9dc2b0cc10 100644 --- a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h +++ b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h @@ -367,7 +367,8 @@ public: ManagementAPIError::toJsonString(false, "rename tenant", "tenant rename conflict")); throw special_keys_api_failure(); } - tenantManagementFutures.push_back(changeTenantConfig(ryw, configMutation.first, configMutation.second)); + tenantManagementFutures.push_back( + changeTenantConfig(ryw, configMutation.first, configMutation.second, &tenantGroupNetTenantDelta)); } for (auto renameMutation : renameMutations) { diff --git a/fdbserver/workloads/TenantManagementWorkload.actor.cpp b/fdbserver/workloads/TenantManagementWorkload.actor.cpp index c23e184ace..d7200d4797 100644 --- a/fdbserver/workloads/TenantManagementWorkload.actor.cpp +++ b/fdbserver/workloads/TenantManagementWorkload.actor.cpp @@ -791,7 +791,7 @@ struct TenantManagementWorkload : TestWorkload { return Void(); } - ACTOR Future renameTenant(Database cx, TenantManagementWorkload* self) { + ACTOR static Future renameTenant(Database cx, TenantManagementWorkload* self) { state OperationType operationType = TenantManagementWorkload::randomOperationType(); state int numTenants = 1; state Reference tr = makeReference(cx); @@ -928,89 +928,6 @@ struct TenantManagementWorkload : TestWorkload { } } - ACTOR Future configureTenant(Database cx, TenantManagementWorkload* self) { - state TenantName tenant = self->chooseTenantName(true); - auto itr = self->createdTenants.find(tenant); - state bool exists = itr != self->createdTenants.end(); - state Reference tr = makeReference(cx); - - state std::map, Optional> configuration; - state Optional newTenantGroup; - state bool hasInvalidOption = deterministicRandom()->random01() < 0.1; - - if (!hasInvalidOption || deterministicRandom()->coinflip()) { - newTenantGroup = self->chooseTenantGroup(); - configuration["tenant_group"_sr] = newTenantGroup; - } - if (hasInvalidOption) { - configuration["invalid_option"_sr] = ""_sr; - hasInvalidOption = true; - } - - state bool hasInvalidSpecialKeyTuple = deterministicRandom()->random01() < 0.05; - - loop { - try { - tr->setOption(FDBTransactionOptions::SPECIAL_KEY_SPACE_ENABLE_WRITES); - for (auto [config, value] : configuration) { - Tuple t; - if (hasInvalidSpecialKeyTuple) { - // Wrong number of items - if (deterministicRandom()->coinflip()) { - int numItems = deterministicRandom()->randomInt(0, 3); - if (numItems > 0) { - t.append(tenant); - } - if (numItems > 1) { - t.append(config).append(""_sr); - } - } - // Wrong data types - else { - if (deterministicRandom()->coinflip()) { - t.append(0).append(config); - } else { - t.append(tenant).append(0); - } - } - } else { - t.append(tenant).append(config); - } - if (value.present()) { - tr->set(self->specialKeysTenantConfigPrefix.withSuffix(t.pack()), value.get()); - } else { - tr->clear(self->specialKeysTenantConfigPrefix.withSuffix(t.pack())); - } - } - - wait(tr->commit()); - - ASSERT(exists); - ASSERT(!hasInvalidOption); - ASSERT(!hasInvalidSpecialKeyTuple); - - self->createdTenants[tenant].tenantGroup = newTenantGroup; - return Void(); - } catch (Error& e) { - state Error error = e; - if (e.code() == error_code_tenant_not_found) { - ASSERT(!exists); - return Void(); - } else if (e.code() == error_code_special_keys_api_failure) { - ASSERT(hasInvalidSpecialKeyTuple || hasInvalidOption); - return Void(); - } - - try { - wait(tr->onError(e)); - } catch (Error&) { - TraceEvent(SevError, "ConfigureTenantFailure").error(error).detail("TenantName", tenant); - return Void(); - } - } - } - } - // Changes the configuration of a tenant ACTOR static Future configureImpl(Reference tr, TenantName tenant, From 3cddeab5c8043bf67b40d2b579abe66f8485cf62 Mon Sep 17 00:00:00 2001 From: Jon Fu Date: Fri, 22 Jul 2022 16:14:25 -0700 Subject: [PATCH 10/20] fix double counting of config mutations --- fdbclient/include/fdbclient/TenantSpecialKeys.actor.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h index 9dc2b0cc10..a5934a63bb 100644 --- a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h +++ b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h @@ -357,10 +357,6 @@ public: if (!tenantsToCreate.empty()) { tenantManagementFutures.push_back(createTenants(ryw, tenantsToCreate, &tenantGroupNetTenantDelta)); } - for (auto configMutation : configMutations) { - tenantManagementFutures.push_back( - changeTenantConfig(ryw, configMutation.first, configMutation.second, &tenantGroupNetTenantDelta)); - } for (auto configMutation : configMutations) { if (renameSet.count(configMutation.first)) { ryw->setSpecialKeySpaceErrorMsg( From 0950d82fe52be3291cdb0b9ca3821d8a37a66608 Mon Sep 17 00:00:00 2001 From: Jon Fu Date: Tue, 26 Jul 2022 11:58:49 -0700 Subject: [PATCH 11/20] add documentation and fix merge errors --- documentation/sphinx/source/special-keys.rst | 1 + .../fdbclient/TenantManagement.actor.h | 2 +- .../TenantManagementWorkload.actor.cpp | 129 ------------------ 3 files changed, 2 insertions(+), 130 deletions(-) diff --git a/documentation/sphinx/source/special-keys.rst b/documentation/sphinx/source/special-keys.rst index 1a278d19b4..1b77b2282e 100644 --- a/documentation/sphinx/source/special-keys.rst +++ b/documentation/sphinx/source/special-keys.rst @@ -209,6 +209,7 @@ that process, and wait for necessary data to be moved away. #. ``\xff\xff/management/options/excluded_locality/force`` Read/write. Setting this key disables safety checks for writes to ``\xff\xff/management/excluded_locality/``. Setting this key only has an effect in the current transaction and is not persisted on commit. #. ``\xff\xff/management/options/failed_locality/force`` Read/write. Setting this key disables safety checks for writes to ``\xff\xff/management/failed_locality/``. Setting this key only has an effect in the current transaction and is not persisted on commit. #. ``\xff\xff/management/tenant/map/`` Read/write. Setting a key in this range to any value will result in a tenant being created with name ````. Clearing a key in this range will delete the tenant with name ````. Reading all or a portion of this range will return the list of tenants currently present in the cluster, excluding any changes in this transaction. Values read in this range will be JSON objects containing the metadata for the associated tenants. +#. ``\xff\xff/management/tenant/rename/`` Read/write. Setting a key in this range to an unused tenant name will result in the tenant with the name ```` to be renamed to the value provided. If the rename operation is a transaction retried in a loop outside of ``fdbcli``, it is possible for the rename to have already occurred, in which case ``tenant_not_found`` or ``tenant_already_exists`` errors may be returned. This can be avoided by checking for the tenant's existence first. An exclusion is syntactically either an ip address (e.g. ``127.0.0.1``), or an ip address and port (e.g. ``127.0.0.1:4500``) or any locality (e.g ``locality_dcid:primary-satellite`` or diff --git a/fdbclient/include/fdbclient/TenantManagement.actor.h b/fdbclient/include/fdbclient/TenantManagement.actor.h index 081763c733..5041036f16 100644 --- a/fdbclient/include/fdbclient/TenantManagement.actor.h +++ b/fdbclient/include/fdbclient/TenantManagement.actor.h @@ -443,7 +443,7 @@ Future renameTenant(Reference db, TenantName oldName, TenantName newNa } } wait(renameTenantTransaction(tr, oldName, newName)); - wait(safeThreadFutureToFuture(tr->commit())); + wait(buggifiedCommit(tr, BUGGIFY_WITH_PROB(0.1))); TraceEvent("RenameTenantSuccess").detail("OldName", oldName).detail("NewName", newName); return Void(); } catch (Error& e) { diff --git a/fdbserver/workloads/TenantManagementWorkload.actor.cpp b/fdbserver/workloads/TenantManagementWorkload.actor.cpp index 65f712d067..8d11c9b2c3 100644 --- a/fdbserver/workloads/TenantManagementWorkload.actor.cpp +++ b/fdbserver/workloads/TenantManagementWorkload.actor.cpp @@ -1057,135 +1057,6 @@ struct TenantManagementWorkload : TestWorkload { } } - // Changes the configuration of a tenant - ACTOR static Future configureImpl(Reference tr, - TenantName tenant, - std::map, Optional> configParameters, - OperationType operationType, - bool specialKeysUseInvalidTuple, - TenantManagementWorkload* self) { - if (operationType == OperationType::SPECIAL_KEYS) { - tr->setOption(FDBTransactionOptions::SPECIAL_KEY_SPACE_ENABLE_WRITES); - for (auto const& [config, value] : configParameters) { - Tuple t; - if (specialKeysUseInvalidTuple) { - // Wrong number of items - if (deterministicRandom()->coinflip()) { - int numItems = deterministicRandom()->randomInt(0, 3); - if (numItems > 0) { - t.append(tenant); - } - if (numItems > 1) { - t.append(config).append(""_sr); - } - } - // Wrong data types - else { - if (deterministicRandom()->coinflip()) { - t.append(0).append(config); - } else { - t.append(tenant).append(0); - } - } - } else { - t.append(tenant).append(config); - } - if (value.present()) { - tr->set(self->specialKeysTenantConfigPrefix.withSuffix(t.pack()), value.get()); - } else { - tr->clear(self->specialKeysTenantConfigPrefix.withSuffix(t.pack())); - } - } - - wait(tr->commit()); - ASSERT(!specialKeysUseInvalidTuple); - } else { - // We don't have a transaction or database variant of this function - ASSERT(false); - } - - return Void(); - } - - ACTOR static Future configureTenant(Database cx, TenantManagementWorkload* self) { - state OperationType operationType = OperationType::SPECIAL_KEYS; - - state TenantName tenant = self->chooseTenantName(true); - auto itr = self->createdTenants.find(tenant); - state bool exists = itr != self->createdTenants.end(); - state Reference tr = makeReference(cx); - - state std::map, Optional> configuration; - state Optional newTenantGroup; - - // If true, the options generated may include an unknown option - state bool hasInvalidOption = deterministicRandom()->random01() < 0.1; - - // True if any tenant group name starts with \xff - state bool hasSystemTenantGroup = false; - - state bool specialKeysUseInvalidTuple = - operationType == OperationType::SPECIAL_KEYS && deterministicRandom()->random01() < 0.1; - - // Generate a tenant group. Sometimes do this at the same time that we include an invalid option to ensure - // that the configure function still fails - if (!hasInvalidOption || deterministicRandom()->coinflip()) { - newTenantGroup = self->chooseTenantGroup(true); - hasSystemTenantGroup = hasSystemTenantGroup || newTenantGroup.orDefault(""_sr).startsWith("\xff"_sr); - configuration["tenant_group"_sr] = newTenantGroup; - } - if (hasInvalidOption) { - configuration["invalid_option"_sr] = ""_sr; - } - - loop { - try { - wait(configureImpl(tr, tenant, configuration, operationType, specialKeysUseInvalidTuple, self)); - - ASSERT(exists); - ASSERT(!hasInvalidOption); - ASSERT(!hasSystemTenantGroup); - ASSERT(!specialKeysUseInvalidTuple); - - auto itr = self->createdTenants.find(tenant); - if (itr->second.tenantGroup.present()) { - auto tenantGroupItr = self->createdTenantGroups.find(itr->second.tenantGroup.get()); - ASSERT(tenantGroupItr != self->createdTenantGroups.end()); - if (--tenantGroupItr->second.tenantCount == 0) { - self->createdTenantGroups.erase(tenantGroupItr); - } - } - if (newTenantGroup.present()) { - self->createdTenantGroups[newTenantGroup.get()].tenantCount++; - } - itr->second.tenantGroup = newTenantGroup; - return Void(); - } catch (Error& e) { - state Error error = e; - if (e.code() == error_code_tenant_not_found) { - ASSERT(!exists); - return Void(); - } else if (e.code() == error_code_special_keys_api_failure) { - ASSERT(hasInvalidOption || specialKeysUseInvalidTuple); - return Void(); - } else if (e.code() == error_code_invalid_tenant_configuration) { - ASSERT(hasInvalidOption); - return Void(); - } else if (e.code() == error_code_invalid_tenant_group_name) { - ASSERT(hasSystemTenantGroup); - return Void(); - } - - try { - wait(tr->onError(e)); - } catch (Error&) { - TraceEvent(SevError, "ConfigureTenantFailure").error(error).detail("TenantName", tenant); - return Void(); - } - } - } - } - Future start(Database const& cx) override { return _start(cx, this); } ACTOR Future _start(Database cx, TenantManagementWorkload* self) { state double start = now(); From fbf566ac4ad6a7d7b29f7fdf968e881678f5a535 Mon Sep 17 00:00:00 2001 From: Jon Fu Date: Tue, 26 Jul 2022 15:53:21 -0400 Subject: [PATCH 12/20] Update documentation/sphinx/source/special-keys.rst Co-authored-by: A.J. Beamon --- documentation/sphinx/source/special-keys.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/sphinx/source/special-keys.rst b/documentation/sphinx/source/special-keys.rst index 1b77b2282e..45b9576f31 100644 --- a/documentation/sphinx/source/special-keys.rst +++ b/documentation/sphinx/source/special-keys.rst @@ -209,7 +209,7 @@ that process, and wait for necessary data to be moved away. #. ``\xff\xff/management/options/excluded_locality/force`` Read/write. Setting this key disables safety checks for writes to ``\xff\xff/management/excluded_locality/``. Setting this key only has an effect in the current transaction and is not persisted on commit. #. ``\xff\xff/management/options/failed_locality/force`` Read/write. Setting this key disables safety checks for writes to ``\xff\xff/management/failed_locality/``. Setting this key only has an effect in the current transaction and is not persisted on commit. #. ``\xff\xff/management/tenant/map/`` Read/write. Setting a key in this range to any value will result in a tenant being created with name ````. Clearing a key in this range will delete the tenant with name ````. Reading all or a portion of this range will return the list of tenants currently present in the cluster, excluding any changes in this transaction. Values read in this range will be JSON objects containing the metadata for the associated tenants. -#. ``\xff\xff/management/tenant/rename/`` Read/write. Setting a key in this range to an unused tenant name will result in the tenant with the name ```` to be renamed to the value provided. If the rename operation is a transaction retried in a loop outside of ``fdbcli``, it is possible for the rename to have already occurred, in which case ``tenant_not_found`` or ``tenant_already_exists`` errors may be returned. This can be avoided by checking for the tenant's existence first. +#. ``\xff\xff/management/tenant/rename/`` Read/write. Setting a key in this range to an unused tenant name will result in the tenant with the name ```` to be renamed to the value provided. If the rename operation is a transaction retried in a loop, it is possible for the rename to be applied twice, in which case ``tenant_not_found`` or ``tenant_already_exists`` errors may be returned. This can be avoided by checking for the tenant's existence first. An exclusion is syntactically either an ip address (e.g. ``127.0.0.1``), or an ip address and port (e.g. ``127.0.0.1:4500``) or any locality (e.g ``locality_dcid:primary-satellite`` or From 6dbaf46ec85fb2524c358bb396db0e462cc053da Mon Sep 17 00:00:00 2001 From: Jon Fu Date: Tue, 26 Jul 2022 13:53:04 -0700 Subject: [PATCH 13/20] restructure test workload impl and map --- fdbcli/TenantCommands.actor.cpp | 6 +- fdbcli/fdbcli.actor.cpp | 2 +- fdbcli/include/fdbcli/fdbcli.actor.h | 2 +- .../TenantManagementWorkload.actor.cpp | 150 +++++++++--------- 4 files changed, 76 insertions(+), 84 deletions(-) diff --git a/fdbcli/TenantCommands.actor.cpp b/fdbcli/TenantCommands.actor.cpp index 035e0571ee..eebc556133 100644 --- a/fdbcli/TenantCommands.actor.cpp +++ b/fdbcli/TenantCommands.actor.cpp @@ -446,15 +446,15 @@ int64_t getTenantId(Value metadata) { } // renametenant command -ACTOR Future renameTenantCommandActor(Reference db, std::vector tokens) { +ACTOR Future renameTenantCommandActor(Reference db, std::vector tokens, int apiVersion) { if (tokens.size() != 3) { printUsage(tokens[0]); return false; } state Reference tr = db->createTransaction(); state Key tenantRenameKey = tenantRenameSpecialKeyRange.begin.withSuffix(tokens[1]); - state Key tenantOldNameKey = tenantMapSpecialKeyRange.begin.withSuffix(tokens[1]); - state Key tenantNewNameKey = tenantMapSpecialKeyRange.begin.withSuffix(tokens[2]); + state Key tenantOldNameKey = tenantMapSpecialKeyRange(apiVersion).begin.withSuffix(tokens[1]); + state Key tenantNewNameKey = tenantMapSpecialKeyRange(apiVersion).begin.withSuffix(tokens[2]); state bool firstTry = true; state int64_t id; loop { diff --git a/fdbcli/fdbcli.actor.cpp b/fdbcli/fdbcli.actor.cpp index d51adcb241..072b11fec0 100644 --- a/fdbcli/fdbcli.actor.cpp +++ b/fdbcli/fdbcli.actor.cpp @@ -1961,7 +1961,7 @@ ACTOR Future cli(CLIOptions opt, LineNoise* plinenoise) { continue; } - bool _result = wait(makeInterruptable(renameTenantCommandActor(db, tokens))); + bool _result = wait(makeInterruptable(renameTenantCommandActor(db, tokens, opt.apiVersion))); if (!_result) is_error = true; continue; diff --git a/fdbcli/include/fdbcli/fdbcli.actor.h b/fdbcli/include/fdbcli/fdbcli.actor.h index 1ea649a805..2b56c216a0 100644 --- a/fdbcli/include/fdbcli/fdbcli.actor.h +++ b/fdbcli/include/fdbcli/fdbcli.actor.h @@ -221,7 +221,7 @@ ACTOR Future profileCommandActor(Database db, std::vector tokens, bool intrans); // renametenant command -ACTOR Future renameTenantCommandActor(Reference db, std::vector tokens); +ACTOR Future renameTenantCommandActor(Reference db, std::vector tokens, int apiVersion); // quota command ACTOR Future quotaCommandActor(Reference db, std::vector tokens); // setclass command diff --git a/fdbserver/workloads/TenantManagementWorkload.actor.cpp b/fdbserver/workloads/TenantManagementWorkload.actor.cpp index 8d11c9b2c3..2937a65ac8 100644 --- a/fdbserver/workloads/TenantManagementWorkload.actor.cpp +++ b/fdbserver/workloads/TenantManagementWorkload.actor.cpp @@ -763,30 +763,65 @@ struct TenantManagementWorkload : TestWorkload { wait(insertTr.onError(e)); } } - } else { - loop { - try { - insertTr.clear(KeyRangeRef(""_sr, "\xff"_sr)); - wait(insertTr.commit()); - break; - } catch (Error& e) { - wait(insertTr.onError(e)); - } - } } return Void(); } ACTOR Future verifyTenantRenames(Database cx, TenantManagementWorkload* self, - std::vector oldTenantNames, - std::vector newTenantNames, + std::map tenantRenames, int numTenants) { - state int tIndex = 0; - for (; tIndex != numTenants; ++tIndex) { - wait(self->verifyTenantRename(cx, self, oldTenantNames[tIndex], newTenantNames[tIndex])); - state TenantName newTenantName = newTenantNames[tIndex]; - wait(self->checkTenantContents(cx, self, newTenantName, self->createdTenants[newTenantName])); + state std::map tenantRenamesCopy = tenantRenames; + state std::map::iterator iter = tenantRenamesCopy.begin(); + for (; iter != tenantRenamesCopy.end(); ++iter) { + wait(self->verifyTenantRename(cx, self, iter->first, iter->second)); + wait(self->checkTenantContents(cx, self, iter->second, self->createdTenants[iter->second])); + } + return Void(); + } + + ACTOR static Future renameImpl(Database cx, + Reference tr, + OperationType operationType, + int numTenants, + std::map tenantRenames, + bool tenantNotFound, + bool tenantExists, + bool tenantOverlap, + TenantManagementWorkload* self) { + if (operationType == OperationType::SPECIAL_KEYS) { + tr->setOption(FDBTransactionOptions::SPECIAL_KEY_SPACE_ENABLE_WRITES); + // state std::map::iterator iter = tenantRenames.begin(); + for (auto& iter : tenantRenames) { + tr->set(self->specialKeysTenantRenamePrefix.withSuffix(iter.first), iter.second); + } + wait(tr->commit()); + } else if (operationType == OperationType::MANAGEMENT_DATABASE) { + ASSERT(tenantRenames.size() == 1); + auto iter = tenantRenames.begin(); + wait(TenantAPI::renameTenant(cx.getReference(), iter->first, iter->second)); + ASSERT(!tenantNotFound && !tenantExists); + } else { // operationType == OperationType::MANAGEMENT_TRANSACTION + tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); + // These types of overlap will also fail as tenantNotFound or tenantExists: + // A->A + // A->B + B->C + // These types of overlap: + // A->B + A->C (Old Name Overlap) + // A->C + B->C (New Name Overlap) + // cannot be detected since everything happens in one commit + // and will not observe each other's changes in time + if (tenantOverlap) { + throw special_keys_api_failure(); + } + std::vector> renameFutures; + // state std::map::iterator iter = tenantRenames.begin(); + for (auto& iter : tenantRenames) { + renameFutures.push_back(success(TenantAPI::renameTenantTransaction(tr, iter.first, iter.second))); + } + wait(waitForAll(renameFutures)); + wait(tr->commit()); + ASSERT(!tenantNotFound && !tenantExists); } return Void(); } @@ -800,8 +835,7 @@ struct TenantManagementWorkload : TestWorkload { numTenants = deterministicRandom()->randomInt(1, 5); } - state std::vector oldTenantNames; - state std::vector newTenantNames; + state std::map tenantRenames; state std::set allTenantNames; // Tenant Error flags @@ -815,8 +849,7 @@ struct TenantManagementWorkload : TestWorkload { TenantName newTenant = self->chooseTenantName(false); tenantOverlap = tenantOverlap || oldTenant == newTenant || allTenantNames.count(oldTenant) || allTenantNames.count(newTenant); - oldTenantNames.push_back(oldTenant); - newTenantNames.push_back(newTenant); + tenantRenames[oldTenant] = newTenant; allTenantNames.insert(oldTenant); allTenantNames.insert(newTenant); if (!self->createdTenants.count(oldTenant)) { @@ -829,81 +862,40 @@ struct TenantManagementWorkload : TestWorkload { loop { try { - if (operationType == OperationType::SPECIAL_KEYS) { - tr->setOption(FDBTransactionOptions::SPECIAL_KEY_SPACE_ENABLE_WRITES); - // Some of the tenant error flags may be true, but it is possible for the commit not to - // throw an error. This is because if we get A->B + A->C - // the special keys uses RYW and only applies the latest op, while the violation - // may have been from one of the earlier ops, or overlapping on the same "old" key - for (int i = 0; i != numTenants; ++i) { - if (std::count(oldTenantNames.begin(), oldTenantNames.end(), oldTenantNames[i]) > 1) { - // Throw instead of committing to avoid mismatch in internal map and actual DB state. - ASSERT(tenantOverlap); - throw special_keys_api_failure(); - } - tr->set(self->specialKeysTenantRenamePrefix.withSuffix(oldTenantNames[i]), newTenantNames[i]); - } - wait(tr->commit()); - } else if (operationType == OperationType::MANAGEMENT_DATABASE) { - ASSERT(oldTenantNames.size() == 1 && newTenantNames.size() == 1); - wait(TenantAPI::renameTenant(cx.getReference(), oldTenantNames[0], newTenantNames[0])); - ASSERT(!tenantNotFound && !tenantExists); - } else { // operationType == OperationType::MANAGEMENT_TRANSACTION - tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); - // These types of overlap will also fail as tenantNotFound or tenantExists: - // A->A - // A->B + B->C - std::vector> renameFutures; - for (int i = 0; i != numTenants; ++i) { - // These types of overlap: - // A->B + A->C (Old Name Overlap) - // A->C + B->C (New Name Overlap) - // cannot be detected since everything happens in one commit - // and will not observe each other's changes in time - if (std::count(oldTenantNames.begin(), oldTenantNames.end(), oldTenantNames[i]) > 1 || - std::count(newTenantNames.begin(), newTenantNames.end(), newTenantNames[i]) > 1) { - ASSERT(tenantOverlap); - throw special_keys_api_failure(); - } - renameFutures.push_back( - success(TenantAPI::renameTenantTransaction(tr, oldTenantNames[i], newTenantNames[i]))); - } - wait(waitForAll(renameFutures)); - wait(tr->commit()); - ASSERT(!tenantNotFound && !tenantExists); - } - wait(self->verifyTenantRenames(cx, self, oldTenantNames, newTenantNames, numTenants)); + wait(renameImpl(cx, + tr, + operationType, + numTenants, + tenantRenames, + tenantNotFound, + tenantExists, + tenantOverlap, + self)); + wait(self->verifyTenantRenames(cx, self, tenantRenames, numTenants)); return Void(); } catch (Error& e) { - if (operationType == OperationType::MANAGEMENT_DATABASE) { - ASSERT(oldTenantNames.size() == 1 && newTenantNames.size() == 1); - } if (e.code() == error_code_tenant_not_found) { TraceEvent("RenameTenantOldTenantNotFound") - .detail("OldTenantNames", describe(oldTenantNames)) - .detail("NewTenantNames", describe(newTenantNames)) + .detail("TenantRenames", describe(tenantRenames)) .detail("CommitUnknownResult", unknownResult); if (unknownResult) { - wait(self->verifyTenantRenames(cx, self, oldTenantNames, newTenantNames, numTenants)); + wait(self->verifyTenantRenames(cx, self, tenantRenames, numTenants)); } else { ASSERT(tenantNotFound); } return Void(); } else if (e.code() == error_code_tenant_already_exists) { TraceEvent("RenameTenantNewTenantAlreadyExists") - .detail("OldTenantNames", describe(oldTenantNames)) - .detail("NewTenantNames", describe(newTenantNames)) + .detail("TenantRenames", describe(tenantRenames)) .detail("CommitUnknownResult", unknownResult); if (unknownResult) { - wait(self->verifyTenantRenames(cx, self, oldTenantNames, newTenantNames, numTenants)); + wait(self->verifyTenantRenames(cx, self, tenantRenames, numTenants)); } else { ASSERT(tenantExists); } return Void(); } else if (e.code() == error_code_special_keys_api_failure) { - TraceEvent("RenameTenantNameConflict") - .detail("OldTenantNames", describe(oldTenantNames)) - .detail("NewTenantNames", describe(newTenantNames)); + TraceEvent("RenameTenantNameConflict").detail("TenantRenames", describe(tenantRenames)); ASSERT(tenantOverlap); return Void(); } else { @@ -913,14 +905,14 @@ struct TenantManagementWorkload : TestWorkload { // already been moved, so account for that and update internal map as needed. if (e.code() == error_code_commit_unknown_result) { TraceEvent("RenameTenantCommitUnknownResult").error(e); + ASSERT(operationType != OperationType::MANAGEMENT_DATABASE); unknownResult = true; } wait(tr->onError(e)); } catch (Error& e) { TraceEvent(SevError, "RenameTenantFailure") .error(e) - .detail("OldTenantNames", describe(oldTenantNames)) - .detail("NewTenantNames", describe(newTenantNames)); + .detail("TenantRenames", describe(tenantRenames)); return Void(); } } From 6ff90c83b14e4e0df8f883fab6a3479459a3ff6b Mon Sep 17 00:00:00 2001 From: Jon Fu Date: Tue, 26 Jul 2022 13:58:19 -0700 Subject: [PATCH 14/20] remove extra parameters --- .../TenantManagementWorkload.actor.cpp | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/fdbserver/workloads/TenantManagementWorkload.actor.cpp b/fdbserver/workloads/TenantManagementWorkload.actor.cpp index 2937a65ac8..0b84ed1d75 100644 --- a/fdbserver/workloads/TenantManagementWorkload.actor.cpp +++ b/fdbserver/workloads/TenantManagementWorkload.actor.cpp @@ -769,8 +769,7 @@ struct TenantManagementWorkload : TestWorkload { ACTOR Future verifyTenantRenames(Database cx, TenantManagementWorkload* self, - std::map tenantRenames, - int numTenants) { + std::map tenantRenames) { state std::map tenantRenamesCopy = tenantRenames; state std::map::iterator iter = tenantRenamesCopy.begin(); for (; iter != tenantRenamesCopy.end(); ++iter) { @@ -783,7 +782,6 @@ struct TenantManagementWorkload : TestWorkload { ACTOR static Future renameImpl(Database cx, Reference tr, OperationType operationType, - int numTenants, std::map tenantRenames, bool tenantNotFound, bool tenantExists, @@ -862,16 +860,9 @@ struct TenantManagementWorkload : TestWorkload { loop { try { - wait(renameImpl(cx, - tr, - operationType, - numTenants, - tenantRenames, - tenantNotFound, - tenantExists, - tenantOverlap, - self)); - wait(self->verifyTenantRenames(cx, self, tenantRenames, numTenants)); + wait(renameImpl( + cx, tr, operationType, tenantRenames, tenantNotFound, tenantExists, tenantOverlap, self)); + wait(self->verifyTenantRenames(cx, self, tenantRenames)); return Void(); } catch (Error& e) { if (e.code() == error_code_tenant_not_found) { @@ -879,7 +870,7 @@ struct TenantManagementWorkload : TestWorkload { .detail("TenantRenames", describe(tenantRenames)) .detail("CommitUnknownResult", unknownResult); if (unknownResult) { - wait(self->verifyTenantRenames(cx, self, tenantRenames, numTenants)); + wait(self->verifyTenantRenames(cx, self, tenantRenames)); } else { ASSERT(tenantNotFound); } @@ -889,7 +880,7 @@ struct TenantManagementWorkload : TestWorkload { .detail("TenantRenames", describe(tenantRenames)) .detail("CommitUnknownResult", unknownResult); if (unknownResult) { - wait(self->verifyTenantRenames(cx, self, tenantRenames, numTenants)); + wait(self->verifyTenantRenames(cx, self, tenantRenames)); } else { ASSERT(tenantExists); } From a3b56ebadcdef66a244a25d7b508317a27a3bc4b Mon Sep 17 00:00:00 2001 From: Jon Fu Date: Tue, 26 Jul 2022 15:04:18 -0700 Subject: [PATCH 15/20] reject overlap in transaction case --- .../TenantManagementWorkload.actor.cpp | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/fdbserver/workloads/TenantManagementWorkload.actor.cpp b/fdbserver/workloads/TenantManagementWorkload.actor.cpp index 0b84ed1d75..64e15a449f 100644 --- a/fdbserver/workloads/TenantManagementWorkload.actor.cpp +++ b/fdbserver/workloads/TenantManagementWorkload.actor.cpp @@ -789,7 +789,6 @@ struct TenantManagementWorkload : TestWorkload { TenantManagementWorkload* self) { if (operationType == OperationType::SPECIAL_KEYS) { tr->setOption(FDBTransactionOptions::SPECIAL_KEY_SPACE_ENABLE_WRITES); - // state std::map::iterator iter = tenantRenames.begin(); for (auto& iter : tenantRenames) { tr->set(self->specialKeysTenantRenamePrefix.withSuffix(iter.first), iter.second); } @@ -801,19 +800,7 @@ struct TenantManagementWorkload : TestWorkload { ASSERT(!tenantNotFound && !tenantExists); } else { // operationType == OperationType::MANAGEMENT_TRANSACTION tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS); - // These types of overlap will also fail as tenantNotFound or tenantExists: - // A->A - // A->B + B->C - // These types of overlap: - // A->B + A->C (Old Name Overlap) - // A->C + B->C (New Name Overlap) - // cannot be detected since everything happens in one commit - // and will not observe each other's changes in time - if (tenantOverlap) { - throw special_keys_api_failure(); - } std::vector> renameFutures; - // state std::map::iterator iter = tenantRenames.begin(); for (auto& iter : tenantRenames) { renameFutures.push_back(success(TenantAPI::renameTenantTransaction(tr, iter.first, iter.second))); } @@ -845,8 +832,23 @@ struct TenantManagementWorkload : TestWorkload { for (int i = 0; i < numTenants; ++i) { TenantName oldTenant = self->chooseTenantName(false); TenantName newTenant = self->chooseTenantName(false); - tenantOverlap = tenantOverlap || oldTenant == newTenant || allTenantNames.count(oldTenant) || - allTenantNames.count(newTenant); + bool checkOverlap = + oldTenant == newTenant || allTenantNames.count(oldTenant) || allTenantNames.count(newTenant); + // reject the rename here if it has overlap and we are doing a transaction operation + // and then pick another combination + if (checkOverlap && operationType == OperationType::MANAGEMENT_TRANSACTION) { + // These types of overlap will also fail as tenantNotFound or tenantExists: + // A->A + // A->B + B->C + // These types of overlap: + // A->B + A->C (Old Name Overlap) + // A->C + B->C (New Name Overlap) + // cannot be detected since everything happens in one commit + // and will not observe each other's changes in time + --i; + continue; + } + tenantOverlap = tenantOverlap || checkOverlap; tenantRenames[oldTenant] = newTenant; allTenantNames.insert(oldTenant); allTenantNames.insert(newTenant); From f0687237fb165221f4dea9e8ff2ed2986b3ca86d Mon Sep 17 00:00:00 2001 From: Jon Fu Date: Tue, 26 Jul 2022 15:41:40 -0700 Subject: [PATCH 16/20] update comments --- fdbserver/workloads/TenantManagementWorkload.actor.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/fdbserver/workloads/TenantManagementWorkload.actor.cpp b/fdbserver/workloads/TenantManagementWorkload.actor.cpp index 64e15a449f..e3866023bf 100644 --- a/fdbserver/workloads/TenantManagementWorkload.actor.cpp +++ b/fdbserver/workloads/TenantManagementWorkload.actor.cpp @@ -834,17 +834,10 @@ struct TenantManagementWorkload : TestWorkload { TenantName newTenant = self->chooseTenantName(false); bool checkOverlap = oldTenant == newTenant || allTenantNames.count(oldTenant) || allTenantNames.count(newTenant); + // renameTenantTransaction does not handle rename collisions: // reject the rename here if it has overlap and we are doing a transaction operation // and then pick another combination if (checkOverlap && operationType == OperationType::MANAGEMENT_TRANSACTION) { - // These types of overlap will also fail as tenantNotFound or tenantExists: - // A->A - // A->B + B->C - // These types of overlap: - // A->B + A->C (Old Name Overlap) - // A->C + B->C (New Name Overlap) - // cannot be detected since everything happens in one commit - // and will not observe each other's changes in time --i; continue; } From d39c0b773afe1a646a8ead956ae1e59ed5cfff7b Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Mon, 25 Jul 2022 09:53:56 -0700 Subject: [PATCH 17/20] Add a limit to the number of tenants that can be created in a cluster --- fdbclient/ClientKnobs.cpp | 4 ++ fdbclient/include/fdbclient/ClientKnobs.h | 3 + fdbclient/include/fdbclient/Tenant.h | 4 +- .../fdbclient/TenantManagement.actor.h | 14 +++++ .../fdbclient/TenantSpecialKeys.actor.h | 25 +++++++-- fdbserver/BlobManager.actor.cpp | 19 ++----- fdbserver/BlobWorker.actor.cpp | 17 ++---- fdbserver/TenantCache.actor.cpp | 4 +- fdbserver/storageserver.actor.cpp | 3 +- .../TenantManagementWorkload.actor.cpp | 56 ++++++++++++++++++- flow/include/flow/error_definitions.h | 1 + 11 files changed, 115 insertions(+), 35 deletions(-) diff --git a/fdbclient/ClientKnobs.cpp b/fdbclient/ClientKnobs.cpp index 0ad5501629..29316ed2b8 100644 --- a/fdbclient/ClientKnobs.cpp +++ b/fdbclient/ClientKnobs.cpp @@ -22,6 +22,7 @@ #include "fdbclient/FDBTypes.h" #include "fdbclient/SystemData.h" #include "fdbclient/Tenant.h" +#include "flow/IRandom.h" #include "flow/UnitTest.h" #define init(...) KNOB_FN(__VA_ARGS__, INIT_ATOMIC_KNOB, INIT_KNOB)(__VA_ARGS__) @@ -289,6 +290,9 @@ void ClientKnobs::initialize(Randomize randomize) { init( CHANGE_QUORUM_BAD_STATE_RETRY_TIMES, 3 ); init( CHANGE_QUORUM_BAD_STATE_RETRY_DELAY, 2.0 ); + // Tenants and Metacluster + init( MAX_TENANTS_PER_CLUSTER, 1e6 ); if ( randomize && BUGGIFY ) MAX_TENANTS_PER_CLUSTER = deterministicRandom()->randomInt(20, 100); + // clang-format on } diff --git a/fdbclient/include/fdbclient/ClientKnobs.h b/fdbclient/include/fdbclient/ClientKnobs.h index 9280cda629..66b8bbc873 100644 --- a/fdbclient/include/fdbclient/ClientKnobs.h +++ b/fdbclient/include/fdbclient/ClientKnobs.h @@ -284,6 +284,9 @@ public: int CHANGE_QUORUM_BAD_STATE_RETRY_TIMES; double CHANGE_QUORUM_BAD_STATE_RETRY_DELAY; + // Tenants and Metacluster + int MAX_TENANTS_PER_CLUSTER; + ClientKnobs(Randomize randomize); void initialize(Randomize randomize); }; diff --git a/fdbclient/include/fdbclient/Tenant.h b/fdbclient/include/fdbclient/Tenant.h index 089eb5365b..cdcc1d0df4 100644 --- a/fdbclient/include/fdbclient/Tenant.h +++ b/fdbclient/include/fdbclient/Tenant.h @@ -99,12 +99,13 @@ struct TenantMetadataSpecification { KeyBackedObjectMap tenantMap; KeyBackedProperty lastTenantId; + KeyBackedBinaryValue tenantCount; KeyBackedSet tenantGroupTenantIndex; KeyBackedObjectMap tenantGroupMap; TenantMetadataSpecification(KeyRef subspace) : tenantMap(subspace.withSuffix("tenant/map/"_sr), IncludeVersion()), - lastTenantId(subspace.withSuffix("tenant/lastId"_sr)), + lastTenantId(subspace.withSuffix("tenant/lastId"_sr)), tenantCount(subspace.withSuffix("tenant/count"_sr)), tenantGroupTenantIndex(subspace.withSuffix("tenant/tenantGroup/tenantIndex/"_sr)), tenantGroupMap(subspace.withSuffix("tenant/tenantGroup/map/"_sr), IncludeVersion()) {} }; @@ -116,6 +117,7 @@ private: public: static inline auto& tenantMap = instance.tenantMap; static inline auto& lastTenantId = instance.lastTenantId; + static inline auto& tenantCount = instance.tenantCount; static inline auto& tenantGroupTenantIndex = instance.tenantGroupTenantIndex; static inline auto& tenantGroupMap = instance.tenantGroupMap; diff --git a/fdbclient/include/fdbclient/TenantManagement.actor.h b/fdbclient/include/fdbclient/TenantManagement.actor.h index db32f1130a..7838db39b5 100644 --- a/fdbclient/include/fdbclient/TenantManagement.actor.h +++ b/fdbclient/include/fdbclient/TenantManagement.actor.h @@ -19,6 +19,7 @@ */ #pragma once +#include "fdbclient/ClientBooleanParams.h" #include "flow/IRandom.h" #if defined(NO_INTELLISENSE) && !defined(FDBCLIENT_TENANT_MANAGEMENT_ACTOR_G_H) #define FDBCLIENT_TENANT_MANAGEMENT_ACTOR_G_H @@ -140,6 +141,16 @@ Future, bool>> createTenantTransaction(Transa } } + // This is idempotent because we only add an entry to the tenant map if it isn't already there + TenantMetadata::tenantCount.atomicOp(tr, 1, MutationRef::AddValue); + + // Read the tenant count after incrementing the counter so that simultaneous attempts to create + // tenants in the same transaction are properly reflected. + int64_t tenantCount = wait(TenantMetadata::tenantCount.getD(tr, Snapshot::False, 0)); + if (tenantCount > CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER) { + throw cluster_no_capacity(); + } + return std::make_pair(tenantEntry, true); } @@ -232,7 +243,10 @@ Future deleteTenantTransaction(Transaction tr, throw tenant_not_empty(); } + // This is idempotent because we only erase an entry from the tenant map if it is present TenantMetadata::tenantMap.erase(tr, name); + TenantMetadata::tenantCount.atomicOp(tr, -1, MutationRef::AddValue); + if (tenantEntry.get().tenantGroup.present()) { TenantMetadata::tenantGroupTenantIndex.erase(tr, Tuple::makeTuple(tenantEntry.get().tenantGroup.get(), name)); diff --git a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h index edefe22020..ece4f6ec6c 100644 --- a/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h +++ b/fdbclient/include/fdbclient/TenantSpecialKeys.actor.h @@ -107,7 +107,8 @@ private: return results; } - ACTOR static Future createTenant( + // Returns true if the tenant was created, false if it already existed + ACTOR static Future createTenant( ReadYourWritesTransaction* ryw, TenantNameRef tenantName, std::vector, Optional>> configMutations, @@ -127,23 +128,39 @@ private: std::pair, bool> entry = wait(TenantAPI::createTenantTransaction(&ryw->getTransaction(), tenantName, tenantEntry)); - return Void(); + return entry.second; } ACTOR static Future createTenants( ReadYourWritesTransaction* ryw, std::map, Optional>>> tenants, std::map* tenantGroupNetTenantDelta) { + state Future tenantCountFuture = + TenantMetadata::tenantCount.getD(&ryw->getTransaction(), Snapshot::False, 0); int64_t _nextId = wait(TenantAPI::getNextTenantId(&ryw->getTransaction())); - int64_t nextId = _nextId; + state int64_t nextId = _nextId; - std::vector> createFutures; + state std::vector> createFutures; for (auto const& [tenant, config] : tenants) { createFutures.push_back(createTenant(ryw, tenant, config, nextId++, tenantGroupNetTenantDelta)); } TenantMetadata::lastTenantId.set(&ryw->getTransaction(), nextId - 1); wait(waitForAll(createFutures)); + + state int numCreatedTenants = 0; + for (auto f : createFutures) { + if (f.get()) { + ++numCreatedTenants; + } + } + + // Check the tenant count here rather than rely on the createTenantTransaction check because we don't have RYW + int64_t tenantCount = wait(tenantCountFuture); + if (tenantCount + numCreatedTenants > CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER) { + throw cluster_no_capacity(); + } + return Void(); } diff --git a/fdbserver/BlobManager.actor.cpp b/fdbserver/BlobManager.actor.cpp index 446ec2e390..c8f8431682 100644 --- a/fdbserver/BlobManager.actor.cpp +++ b/fdbserver/BlobManager.actor.cpp @@ -967,19 +967,12 @@ ACTOR Future monitorClientRanges(Reference bmData) { } else { state KeyBackedRangeResult> tenantResults; wait(store(tenantResults, - TenantMetadata::tenantMap.getRange( - tr, Optional(), Optional(), CLIENT_KNOBS->TOO_MANY))); - ASSERT_WE_THINK(!tenantResults.more && tenantResults.results.size() < CLIENT_KNOBS->TOO_MANY); - if (tenantResults.more || tenantResults.results.size() >= CLIENT_KNOBS->TOO_MANY) { - TraceEvent(SevError, "BlobManagerTooManyTenants", bmData->id) - .detail("Epoch", bmData->epoch) - .detail("TenantCount", tenantResults.results.size()); - wait(delay(600)); - if (bmData->iAmReplaced.canBeSet()) { - bmData->iAmReplaced.sendError(internal_error()); - } - throw internal_error(); - } + TenantMetadata::tenantMap.getRange(tr, + Optional(), + Optional(), + CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER + 1))); + ASSERT(tenantResults.results.size() <= CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER && + !tenantResults.more); std::vector prefixes; for (auto& it : tenantResults.results) { diff --git a/fdbserver/BlobWorker.actor.cpp b/fdbserver/BlobWorker.actor.cpp index 568c12ebef..533dac0aaa 100644 --- a/fdbserver/BlobWorker.actor.cpp +++ b/fdbserver/BlobWorker.actor.cpp @@ -3999,18 +3999,11 @@ ACTOR Future monitorTenants(Reference bwData) { tr->setOption(FDBTransactionOptions::PRIORITY_SYSTEM_IMMEDIATE); state KeyBackedRangeResult> tenantResults; wait(store(tenantResults, - TenantMetadata::tenantMap.getRange( - tr, Optional(), Optional(), CLIENT_KNOBS->TOO_MANY))); - ASSERT_WE_THINK(!tenantResults.more && tenantResults.results.size() < CLIENT_KNOBS->TOO_MANY); - if (tenantResults.more || tenantResults.results.size() >= CLIENT_KNOBS->TOO_MANY) { - TraceEvent(SevError, "BlobWorkerTooManyTenants", bwData->id) - .detail("TenantCount", tenantResults.results.size()); - wait(delay(600)); - if (bwData->fatalError.canBeSet()) { - bwData->fatalError.sendError(internal_error()); - } - throw internal_error(); - } + TenantMetadata::tenantMap.getRange(tr, + Optional(), + Optional(), + CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER + 1))); + ASSERT(tenantResults.results.size() <= CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER && !tenantResults.more); std::vector> tenants; for (auto& it : tenantResults.results) { diff --git a/fdbserver/TenantCache.actor.cpp b/fdbserver/TenantCache.actor.cpp index dffda39e4e..2d3cb5cde0 100644 --- a/fdbserver/TenantCache.actor.cpp +++ b/fdbserver/TenantCache.actor.cpp @@ -33,8 +33,8 @@ class TenantCacheImpl { KeyBackedRangeResult> tenantList = wait(TenantMetadata::tenantMap.getRange( - tr, Optional(), Optional(), CLIENT_KNOBS->TOO_MANY)); - ASSERT(!tenantList.more && tenantList.results.size() < CLIENT_KNOBS->TOO_MANY); + tr, Optional(), Optional(), CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER + 1)); + ASSERT(tenantList.results.size() <= CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER && !tenantList.more); return tenantList.results; } diff --git a/fdbserver/storageserver.actor.cpp b/fdbserver/storageserver.actor.cpp index 84e6771e74..a9aac7af82 100644 --- a/fdbserver/storageserver.actor.cpp +++ b/fdbserver/storageserver.actor.cpp @@ -10127,7 +10127,8 @@ ACTOR Future initTenantMap(StorageServer* self) { // when SSs store only the local tenants KeyBackedRangeResult> entries = wait(TenantMetadata::tenantMap.getRange( - tr, Optional(), Optional(), CLIENT_KNOBS->TOO_MANY)); + tr, Optional(), Optional(), CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER + 1)); + ASSERT(entries.results.size() <= CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER && !entries.more); TraceEvent("InitTenantMap", self->thisServerID) .detail("Version", version) diff --git a/fdbserver/workloads/TenantManagementWorkload.actor.cpp b/fdbserver/workloads/TenantManagementWorkload.actor.cpp index f09c2daea9..49c23205ff 100644 --- a/fdbserver/workloads/TenantManagementWorkload.actor.cpp +++ b/fdbserver/workloads/TenantManagementWorkload.actor.cpp @@ -20,7 +20,9 @@ #include #include +#include "fdbclient/ClientBooleanParams.h" #include "fdbclient/FDBOptions.g.h" +#include "fdbclient/ReadYourWrites.h" #include "fdbclient/RunTransaction.actor.h" #include "fdbclient/TenantManagement.actor.h" #include "fdbclient/TenantSpecialKeys.actor.h" @@ -194,6 +196,7 @@ struct TenantManagementWorkload : TestWorkload { // True if any tenant group name starts with \xff state bool hasSystemTenantGroup = false; + state int newTenants = 0; state std::map tenantsToCreate; for (int i = 0; i < numTenants; ++i) { TenantName tenant = self->chooseTenantName(true); @@ -203,21 +206,39 @@ struct TenantManagementWorkload : TestWorkload { TenantMapEntry entry; entry.tenantGroup = self->chooseTenantGroup(true); - tenantsToCreate[tenant] = entry; - alreadyExists = alreadyExists || self->createdTenants.count(tenant); + if (self->createdTenants.count(tenant)) { + alreadyExists = true; + } else if (!tenantsToCreate.count(tenant)) { + ++newTenants; + } + + tenantsToCreate[tenant] = entry; hasSystemTenant = hasSystemTenant || tenant.startsWith("\xff"_sr); hasSystemTenantGroup = hasSystemTenantGroup || entry.tenantGroup.orDefault(""_sr).startsWith("\xff"_sr); } state Reference tr = makeReference(cx); + state int64_t minTenantCount = std::numeric_limits::max(); + state int64_t finalTenantCount = 0; loop { try { + if (operationType != OperationType::MANAGEMENT_DATABASE) { + tr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS); + wait(store(finalTenantCount, TenantMetadata::tenantCount.getD(tr, Snapshot::False, 0))); + minTenantCount = std::min(finalTenantCount, minTenantCount); + } + wait(createImpl(cx, tr, tenantsToCreate, operationType, self)); if (operationType == OperationType::MANAGEMENT_DATABASE) { ASSERT(!alreadyExists); + } else { + // Make sure that we had capacity to create the tenants. This cannot be validated for database + // operations because we cannot determine the tenant count in the same transaction that the tenant + // is created + ASSERT(minTenantCount + newTenants <= CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER); } // It is not legal to create a tenant or tenant group starting with \xff @@ -294,6 +315,13 @@ struct TenantManagementWorkload : TestWorkload { } else if (e.code() == error_code_invalid_tenant_group_name) { ASSERT(hasSystemTenantGroup); return Void(); + } else if (e.code() == error_code_cluster_no_capacity) { + // Confirm that we overshot our capacity. This check cannot be done for database operations + // because we cannot transactionally get the tenant count with the creation. + if (operationType != OperationType::MANAGEMENT_DATABASE) { + ASSERT(finalTenantCount + newTenants > CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER); + } + return Void(); } // Database-based operations should not need to be retried @@ -972,6 +1000,26 @@ struct TenantManagementWorkload : TestWorkload { return Void(); } + // Verify that the tenant count matches the actual number of tenants in the cluster and that we haven't created too + // many + ACTOR static Future checkTenantCount(Database cx) { + state Reference tr = cx->createTransaction(); + loop { + try { + tr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS); + state int64_t tenantCount = wait(TenantMetadata::tenantCount.getD(tr, Snapshot::False, 0)); + KeyBackedRangeResult> tenants = + wait(TenantMetadata::tenantMap.getRange(tr, {}, {}, CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER + 1)); + + ASSERT(tenants.results.size() == tenantCount && !tenants.more); + ASSERT(tenantCount <= CLIENT_KNOBS->MAX_TENANTS_PER_CLUSTER); + return Void(); + } catch (Error& e) { + wait(tr->onError(e)); + } + } + } + // Verify that the set of tenants in the database matches our local state ACTOR static Future compareTenants(Database cx, TenantManagementWorkload* self) { state std::map::iterator localItr = self->createdTenants.begin(); @@ -1094,6 +1142,10 @@ struct TenantManagementWorkload : TestWorkload { } } + if (self->clientId == 0) { + wait(checkTenantCount(cx)); + } + wait(compareTenants(cx, self) && compareTenantGroups(cx, self)); return true; } diff --git a/flow/include/flow/error_definitions.h b/flow/include/flow/error_definitions.h index 56520c8c15..bc75885925 100755 --- a/flow/include/flow/error_definitions.h +++ b/flow/include/flow/error_definitions.h @@ -235,6 +235,7 @@ ERROR( unknown_tenant, 2137, "Tenant is not available from this server" ) ERROR( illegal_tenant_access, 2138, "Illegal tenant access" ) ERROR( invalid_tenant_group_name, 2139, "Tenant group name cannot begin with \\xff" ) ERROR( invalid_tenant_configuration, 2140, "Tenant configuration is invalid" ) +ERROR( cluster_no_capacity, 2141, "Cluster does not have capacity to perform the specified operation" ) // 2200 - errors from bindings and official APIs ERROR( api_version_unset, 2200, "API version is not set" ) From 38d2d3900117b931678e9a242a8f8aeb5f4811e2 Mon Sep 17 00:00:00 2001 From: Jingyu Zhou Date: Wed, 27 Jul 2022 09:51:34 -0700 Subject: [PATCH 18/20] Add 7.1.16, 7.1.17 release notes --- .../source/release-notes/release-notes-710.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/documentation/sphinx/source/release-notes/release-notes-710.rst b/documentation/sphinx/source/release-notes/release-notes-710.rst index d9312f3781..1cd51ad968 100644 --- a/documentation/sphinx/source/release-notes/release-notes-710.rst +++ b/documentation/sphinx/source/release-notes/release-notes-710.rst @@ -2,6 +2,22 @@ Release Notes ############# +7.1.17 +====== +* Same as 7.1.16 release with AVX enabled. + +7.1.16 +====== +* Released with AVX disabled. +* Fixed a crash bug when cluster controller shuts down. `(PR #7706) `_ +* Fixed a storage server failure when getReadVersion returns an error. `(PR #7688) `_ +* Fixed unbounded status json generation. `(PR #7680) `_ +* Fixed ScopeEventFieldTypeMismatch error for TLogMetrics. `(PR #7640) `_ +* Added getMappedRange latency metrics. `(PR #7632) `_ +* Fixed a version vector performance bug due to not updating client side tag cache. `(PR #7616) `_ +* Fixed DiskReadSeconds and DiskWriteSeconds calculaion in ProcessMetrics. `(PR #7609) `_ +* Added Rocksdb compression and data size stats. `(PR #7596) `_ + 7.1.15 ====== * Same as 7.1.14 release with AVX enabled. From e9e388db994175394f68d425e57353fd26725ec3 Mon Sep 17 00:00:00 2001 From: Jon Fu Date: Wed, 27 Jul 2022 15:13:09 -0700 Subject: [PATCH 19/20] fix formatting --- fdbclient/include/fdbclient/TenantManagement.actor.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/fdbclient/include/fdbclient/TenantManagement.actor.h b/fdbclient/include/fdbclient/TenantManagement.actor.h index bc80c704c0..b9e26d0df7 100644 --- a/fdbclient/include/fdbclient/TenantManagement.actor.h +++ b/fdbclient/include/fdbclient/TenantManagement.actor.h @@ -401,10 +401,8 @@ Future renameTenantTransaction(Transaction tr, TenantNameRef oldName, Tena // 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)); + TenantMetadata::tenantGroupTenantIndex.erase(tr, Tuple::makeTuple(oldEntry.get().tenantGroup.get(), oldName)); + TenantMetadata::tenantGroupTenantIndex.insert(tr, Tuple::makeTuple(oldEntry.get().tenantGroup.get(), newName)); } return Void(); From bcf022575c83fe44eaa854fd003df0f0fad0d7af Mon Sep 17 00:00:00 2001 From: Xiaoxi Wang Date: Wed, 27 Jul 2022 15:55:38 -0700 Subject: [PATCH 20/20] fix storage_metadata disappear because other attribution is missed --- fdbserver/Status.actor.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/fdbserver/Status.actor.cpp b/fdbserver/Status.actor.cpp index 60d527b483..11ae0cabac 100644 --- a/fdbserver/Status.actor.cpp +++ b/fdbserver/Status.actor.cpp @@ -487,6 +487,11 @@ struct RolesInfo { double dataLagSeconds = -1.0; obj["id"] = iface.id().shortString(); obj["role"] = role; + if (iface.metadata.present()) { + obj["storage_metadata"] = iface.metadata.get().toJSON(); + // printf("%s\n", metadataObj.getJson().c_str()); + } + try { TraceEventFields const& storageMetrics = metrics.at("StorageMetrics"); @@ -594,14 +599,12 @@ struct RolesInfo { } } - if (iface.metadata.present()) { - obj["storage_metadata"] = iface.metadata.get().toJSON(); - // printf("%s\n", metadataObj.getJson().c_str()); - } - } catch (Error& e) { if (e.code() != error_code_attribute_not_found) throw e; + else { + TraceEvent(SevWarnAlways, "StorageServerStatusJson").error(e); + } } if (pDataLagSeconds) {