-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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.
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
database configuration error reporting #12378
Conversation
Co-authored-by: aider (vertex_ai/gemini-2.5-pro) <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Result of foundationdb-pr on Linux RHEL 9
|
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Result of foundationdb-pr-clang on Linux RHEL 9
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please clang-format the code.
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-clang on Linux RHEL 9
|
Result of foundationdb-pr on Linux RHEL 9
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Result of foundationdb-pr-clang on Linux RHEL 9
|
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr on Linux RHEL 9
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
(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); |
There was a problem hiding this comment.
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.
Should I update this to write to stdout?
…On Mon, Sep 22, 2025 at 2:34 PM Jingyu Zhou ***@***.***> wrote:
EXTERNAL EMAIL
Secured by Check Point
<https://www.checkpoint.com/harmony/email-security/email-office?friew>
***@***.**** commented on this pull request.
------------------------------
In fdbclient/DatabaseConfiguration.cpp
<#12378 (comment)>:
> - 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);
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.
—
Reply to this email directly, view it on GitHub
<#12378 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKQYR4O4WG24PBLXRHKS2L3UA6LDAVCNFSM6AAAAACG7IOMBWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTENJUGUZTCMBWGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
I think we should keep writing to
I.e., when the variable/knob is set, printing to |
Co-authored-by: aider (vertex_ai/gemini-2.5-pro) <[email protected]>
Co-authored-by: aider (vertex_ai/gemini-2.5-pro) <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The output to stderr
needs to be changed such that fdbserver
won't by default print the message.
…of database configuration test failures in fdbcli
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
LGTM
Result of foundationdb-pr-clang on Linux RHEL 9
|
Result of foundationdb-pr on Linux RHEL 9
|
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-clang on Linux RHEL 9
|
Result of foundationdb-pr on Linux RHEL 9
|
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Problem: fdbcli makes many checks to determine if a configuration change is valid, but if any of those tests fail, it does not give any more information to the user than "ERROR: These changes would make the configuration invalid". This change outputs any failed test and the returning boolean value to allow informed debugging. This is an enhancement, not a bug fix.
Tests: I ran valid and invalid configurations via
fileconfigure regions-bad.json
andfiledconfigure regions-valid.json
which returned expected output. This change only impacts administrative use of fdbcli, so should have no performance impact on the database itself.regions-bad.json:
regions-valid.json
Code-Reviewer Section
The general pull request guidelines can be found here.
Please check each of the following things and check all boxes before accepting a PR.
For Release-Branches
If this PR is made against a release-branch, please also check the following:
release-branch
ormain
if this is the youngest branch)