wbtiger
/
foundationdb
mirror of https://github.com/apple/foundationdb.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Make overwriteProxiesCount a function of conf keys (#6645) 

* Improve assert diagnostics

* Make overwriteProxiesCount a deterministic function of conf keys

Previously, the final values for grv_proxy count, commit_proxy count,
and proxy count were derived from already-derived values from
overwriteProxiesCount. Instead, we should only look at the conf keys.

Also treat these keys as set to -1 if absent

* Add unit test

Illustrates how overwriteCommitProxy is not a function of conf keys.
Passes after change and fails before
This commit is contained in:
Andrew Noyes 2022-03-23 17:15:28 -07:00 committed by GitHub
parent 8f3dbe2575
commit 977fab2089
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 34 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
fdbclient/DatabaseConfiguration.cpp
View File

@ -23,6 +23,7 @@
#include "flow/ITrace.h"
#include "flow/Trace.h"
#include "flow/genericactors.actor.h"
#include "flow/UnitTest.h"
DatabaseConfiguration::DatabaseConfiguration() {
resetInternal();
@ -490,9 +491,9 @@ void DatabaseConfiguration::overwriteProxiesCount() {
Optional<ValueRef> optGrvProxies = DatabaseConfiguration::get(grvProxiesKey);
Optional<ValueRef> optProxies = DatabaseConfiguration::get(proxiesKey);
const int mutableGrvProxyCount = optGrvProxies.present() ? toInt(optGrvProxies.get()) : 0;
const int mutableCommitProxyCount = optCommitProxies.present() ? toInt(optCommitProxies.get()) : 0;
const int mutableProxiesCount = optProxies.present() ? toInt(optProxies.get()) : 0;
const int mutableGrvProxyCount = optGrvProxies.present() ? toInt(optGrvProxies.get()) : -1;
const int mutableCommitProxyCount = optCommitProxies.present() ? toInt(optCommitProxies.get()) : -1;
const int mutableProxiesCount = optProxies.present() ? toInt(optProxies.get()) : -1;
if (mutableProxiesCount > 1) {
TraceEvent(SevDebug, "OverwriteProxiesCount")
@ -502,23 +503,23 @@ void DatabaseConfiguration::overwriteProxiesCount() {
.detail("MutableGrvCPCount", mutableGrvProxyCount)
.detail("MutableProxiesCount", mutableProxiesCount);
if (grvProxyCount == -1 && commitProxyCount > 0) {
if (mutableProxiesCount > commitProxyCount) {
grvProxyCount = mutableProxiesCount - commitProxyCount;
if (mutableGrvProxyCount == -1 && mutableCommitProxyCount > 0) {
if (mutableProxiesCount > mutableCommitProxyCount) {
grvProxyCount = mutableProxiesCount - mutableCommitProxyCount;
} else {
// invalid configuration; provision min GrvProxies
grvProxyCount = 1;
commitProxyCount = mutableProxiesCount - 1;
}
} else if (grvProxyCount > 0 && commitProxyCount == -1) {
if (mutableProxiesCount > grvProxyCount) {
} else if (mutableGrvProxyCount > 0 && mutableCommitProxyCount == -1) {
if (mutableProxiesCount > mutableGrvProxyCount) {
commitProxyCount = mutableProxiesCount - grvProxyCount;
} else {
// invalid configuration; provision min CommitProxies
commitProxyCount = 1;
grvProxyCount = mutableProxiesCount - 1;
}
} else if (grvProxyCount == -1 && commitProxyCount == -1) {
} else if (mutableGrvProxyCount == -1 && mutableCommitProxyCount == -1) {
// Use DEFAULT_COMMIT_GRV_PROXIES_RATIO to split proxies between Grv & Commit proxies
const int derivedGrvProxyCount =
std::max(1,
@ -825,3 +826,21 @@ bool DatabaseConfiguration::isOverridden(std::string key) const {
return false;
}
TEST_CASE("/fdbclient/databaseConfiguration/overwriteCommitProxy") {
DatabaseConfiguration conf1;
conf1.applyMutation(MutationRef(MutationRef::SetValue, "\xff/conf/grv_proxies"_sr, "5"_sr));
conf1.applyMutation(MutationRef(MutationRef::SetValue, "\xff/conf/proxies"_sr, "10"_sr));
conf1.applyMutation(MutationRef(MutationRef::SetValue, "\xff/conf/grv_proxies"_sr, "-1"_sr));
conf1.applyMutation(MutationRef(MutationRef::SetValue, "\xff/conf/commit_proxies"_sr, "-1"_sr));
DatabaseConfiguration conf2;
conf2.applyMutation(MutationRef(MutationRef::SetValue, "\xff/conf/proxies"_sr, "10"_sr));
conf2.applyMutation(MutationRef(MutationRef::SetValue, "\xff/conf/grv_proxies"_sr, "-1"_sr));
conf2.applyMutation(MutationRef(MutationRef::SetValue, "\xff/conf/commit_proxies"_sr, "-1"_sr));
ASSERT(conf1 == conf2);
ASSERT(conf1.getDesiredCommitProxies() == conf2.getDesiredCommitProxies());
return Void();
}

12
fdbserver/ClusterRecovery.actor.cpp
View File

@ -1477,12 +1477,12 @@ ACTOR Future<Void> clusterRecoveryCore(Reference<ClusterRecoveryData> self) {
recoverAndEndEpoch.cancel();
ASSERT(self->commitProxies.size() <= self->configuration.getDesiredCommitProxies());
ASSERT(self->commitProxies.size() >= 1);
ASSERT(self->grvProxies.size() <= self->configuration.getDesiredGrvProxies());
ASSERT(self->grvProxies.size() >= 1);
ASSERT(self->resolvers.size() <= self->configuration.getDesiredResolvers());
ASSERT(self->resolvers.size() >= 1);
ASSERT_LE(self->commitProxies.size(), self->configuration.getDesiredCommitProxies());
ASSERT_GE(self->commitProxies.size(), 1);
ASSERT_LE(self->grvProxies.size(), self->configuration.getDesiredGrvProxies());
ASSERT_GE(self->grvProxies.size(), 1);
ASSERT_LE(self->resolvers.size(), self->configuration.getDesiredResolvers());
ASSERT_GE(self->resolvers.size(), 1);
self->recoveryState = RecoveryState::RECOVERY_TRANSACTION;
TraceEvent(getRecoveryEventName(ClusterRecoveryEventType::CLUSTER_RECOVERY_STATE_EVENT_NAME).c_str(), self->dbgid)