Skip to content

database configuration error reporting #12378

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Sep 26, 2025
Merged

database configuration error reporting #12378

Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1d1da09
refactor: Improve DatabaseConfiguration::isValid() debuggability
dgulinobw Sep 18, 2025
e50277e
Refactor: Only log `log_test` failures to stderr
dgulinobw Sep 18, 2025
59e7408
added comments to document
dgulinobw Sep 19, 2025
664ed29
ran clang-format DatabaseConfiguration.cpp
dgulinobw Sep 19, 2025
9a34618
docs: Clarify fprint call condition in isValid()
dgulinobw Sep 23, 2025
2f74f1f
refactor: Add boolean parameter to isValid to conditionally print log…
dgulinobw Sep 23, 2025
a8008de
Refactor: Add print_invalid param to DatabaseConfiguration::isValid
dgulinobw Sep 23, 2025
2046c43
chore: Add comment about print_invalid parameter
dgulinobw Sep 23, 2025
d148da6
chore: Remove fulfilled AI instruction comment
dgulinobw Sep 23, 2025
ff70bad
added client knob cli_print_invalid_configuration to enable printing …
dgulinobw Sep 25, 2025
5b64318
ran clang-format
dgulinobw Sep 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 34 additions & 18 deletions fdbclient/DatabaseConfiguration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* limitations under the License.
*/

#include <cstdio>
#include "fdbclient/DatabaseConfiguration.h"
#include "fdbclient/FDBTypes.h"
#include "fdbclient/SystemData.h"
Expand Down Expand Up @@ -210,27 +211,42 @@ void DatabaseConfiguration::setDefaultReplicationPolicy() {
}

bool DatabaseConfiguration::isValid() const {
if (!(initialized && tLogWriteAntiQuorum >= 0 && tLogWriteAntiQuorum <= tLogReplicationFactor / 2 &&
tLogReplicationFactor >= 1 && storageTeamSize >= 1 && getDesiredCommitProxies() >= 1 &&
getDesiredGrvProxies() >= 1 && getDesiredLogs() >= 1 && getDesiredResolvers() >= 1 &&
tLogVersion != TLogVersion::UNSET && tLogVersion >= TLogVersion::MIN_RECRUITABLE &&
tLogVersion <= TLogVersion::MAX_SUPPORTED && tLogDataStoreType != KeyValueStoreType::END &&
tLogSpillType != TLogSpillType::UNSET &&
!(tLogSpillType == TLogSpillType::REFERENCE && tLogVersion < TLogVersion::V3) &&
storageServerStoreType != KeyValueStoreType::END && autoCommitProxyCount >= 1 && autoGrvProxyCount >= 1 &&
autoResolverCount >= 1 && autoDesiredTLogCount >= 1 && storagePolicy && tLogPolicy &&
getDesiredRemoteLogs() >= 1 && remoteTLogReplicationFactor >= 0 && repopulateRegionAntiQuorum >= 0 &&
repopulateRegionAntiQuorum <= 1 && usableRegions >= 1 && usableRegions <= 2 && regions.size() <= 2 &&
(usableRegions == 1 || regions.size() == 2) && (regions.size() == 0 || regions[0].priority >= 0) &&
(regions.size() == 0 || tLogPolicy->info() != "dcid^2 x zoneid^2 x 1") &&
auto log_test = [](const char* text, bool val) {
if (!val) {
fprintf(stderr, "%s: false\n", text);
Copy link
Contributor

@jzhou77 jzhou77 Sep 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

many ctest failures are due to output in stderr. It seems fdbserver also calls this function and in the context of fdbserver, we don't want to write anything to the stderr unless it's a real problem. For fdbcli, writing to stderr is fine.

Specifically, fdbmonitor considers stderr as Severity="40" events:

>>>>>>>>>>>>>>>>>>>> Contents of /root/cbuild_output/tmp/RDMKwZPrRUgvtZga/log/fdbmonitor.log:
Time="1758565723.674179" Severity="10" LogGroup="default" Process="fdbmonitor": Started FoundationDB Process Monitor 7.4 (v7.4.0)
Time="1758565723.674372" Severity="10" LogGroup="default" Process="fdbmonitor": Watching conf file /root/cbuild_output/tmp/RDMKwZPrRUgvtZga/etc/foundationdb.conf
Time="1758565723.674381" Severity="10" LogGroup="default" Process="fdbmonitor": Watching conf dir /root/cbuild_output/tmp/RDMKwZPrRUgvtZga/etc/ (2)
Time="1758565723.674399" Severity="10" LogGroup="default" Process="fdbmonitor": Loading configuration /root/cbuild_output/tmp/RDMKwZPrRUgvtZga/etc/foundationdb.conf
Time="1758565723.674496" Severity="10" LogGroup="default" Process="fdbmonitor": Starting fdbserver.60791
Time="1758565723.674733" Severity="10" LogGroup="default" Process="fdbserver.60791": Launching /root/cbuild_output/bin/fdbserver (144481) for fdbserver.60791
Time="1758565723.682948" Severity="10" LogGroup="default" Process="fdbserver.60791": ZoneId set to 0, dcId to [not set]
Time="1758565723.706705" Severity="10" LogGroup="default" Process="fdbserver.60791": FDBD joined cluster.
Time="1758565724.733660" Severity="40" LogGroup="default" Process="fdbserver.60791": initialized: false
Time="1758565726.681194" Severity="40" LogGroup="default" Process="fdbserver.60791": initialized: false
Time="1758565729.664410" Severity="20" LogGroup="default" Process="fdbserver.60791": Process 144481 exited 0, restarting in 0 seconds
Time="1758565729.664723" Severity="10" LogGroup="default" Process="fdbserver.60791": Launching /root/cbuild_output/bin/fdbserver (144525) for fdbserver.60791
Time="1758565729.672800" Severity="10" LogGroup="default" Process="fdbserver.60791": ZoneId set to 0, dcId to [not set]
Time="1758565729.690314" Severity="10" LogGroup="default" Process="fdbserver.60791": FDBD joined cluster.

}
return val;
};
//LOG_TEST(expr) takes an expression that returns a boolean. If the boolean == false, the
//expression and it's boolean return value will be printed.
#define LOG_TEST(expr) log_test(#expr, (expr))
if (!(LOG_TEST(initialized) && LOG_TEST(tLogWriteAntiQuorum >= 0) &&
LOG_TEST(tLogWriteAntiQuorum <= tLogReplicationFactor / 2) && LOG_TEST(tLogReplicationFactor >= 1) &&
LOG_TEST(storageTeamSize >= 1) && LOG_TEST(getDesiredCommitProxies() >= 1) &&
LOG_TEST(getDesiredGrvProxies() >= 1) && LOG_TEST(getDesiredLogs() >= 1) &&
LOG_TEST(getDesiredResolvers() >= 1) && LOG_TEST(tLogVersion != TLogVersion::UNSET) &&
LOG_TEST(tLogVersion >= TLogVersion::MIN_RECRUITABLE) &&
LOG_TEST(tLogVersion <= TLogVersion::MAX_SUPPORTED) && LOG_TEST(tLogDataStoreType != KeyValueStoreType::END) &&
LOG_TEST(tLogSpillType != TLogSpillType::UNSET) &&
LOG_TEST(!(tLogSpillType == TLogSpillType::REFERENCE && tLogVersion < TLogVersion::V3)) &&
LOG_TEST(storageServerStoreType != KeyValueStoreType::END) && LOG_TEST(autoCommitProxyCount >= 1) &&
LOG_TEST(autoGrvProxyCount >= 1) && LOG_TEST(autoResolverCount >= 1) &&
LOG_TEST(autoDesiredTLogCount >= 1) && LOG_TEST(!!storagePolicy) && LOG_TEST(!!tLogPolicy) &&
LOG_TEST(getDesiredRemoteLogs() >= 1) && LOG_TEST(remoteTLogReplicationFactor >= 0) &&
LOG_TEST(repopulateRegionAntiQuorum >= 0) && LOG_TEST(repopulateRegionAntiQuorum <= 1) &&
LOG_TEST(usableRegions >= 1) && LOG_TEST(usableRegions <= 2) && LOG_TEST(regions.size() <= 2) &&
LOG_TEST((usableRegions == 1 || regions.size() == 2)) &&
LOG_TEST((regions.size() == 0 || regions[0].priority >= 0)) &&
LOG_TEST((regions.size() == 0 || tLogPolicy->info() != "dcid^2 x zoneid^2 x 1")) &&
// We cannot specify regions with three_datacenter replication
(perpetualStorageWiggleSpeed == 0 || perpetualStorageWiggleSpeed == 1) &&
isValidPerpetualStorageWiggleLocality(perpetualStorageWiggleLocality) &&
storageMigrationType != StorageMigrationType::UNSET && tenantMode >= TenantMode::DISABLED &&
tenantMode < TenantMode::END && encryptionAtRestMode >= EncryptionAtRestMode::DISABLED &&
encryptionAtRestMode < EncryptionAtRestMode::END)) {
LOG_TEST((perpetualStorageWiggleSpeed == 0 || perpetualStorageWiggleSpeed == 1)) &&
LOG_TEST(isValidPerpetualStorageWiggleLocality(perpetualStorageWiggleLocality)) &&
LOG_TEST(storageMigrationType != StorageMigrationType::UNSET) && LOG_TEST(tenantMode >= TenantMode::DISABLED) &&
LOG_TEST(tenantMode < TenantMode::END) && LOG_TEST(encryptionAtRestMode >= EncryptionAtRestMode::DISABLED) &&
LOG_TEST(encryptionAtRestMode < EncryptionAtRestMode::END))) {
return false;
}
#undef LOG_TEST
std::set<Key> dcIds;
dcIds.insert(Key());
for (auto& r : regions) {
Expand Down