mirror of
https://github.com/XRPLF/rippled.git
synced 2026-08-21 22:30:57 +00:00
fix(telemetry): correct handshake throw and quorum sentinel (WP-A1)
Three defects found reviewing the WP-A1 commit: - Handshake.cpp moved a std::string into std::runtime_error, which has no rvalue constructor. The move never happened and clang-tidy rejects it under performance-move-const-arg, so CI would fail even though the local hook only runs clang-tidy with TIDY=1. Takes the message by const reference instead, and drops the <utility> include that existed only for that move. - ValidatorList disables quorum by returning SIZE_MAX. Casting that to int64_t wrapped it to -1, so the headroom panel computed 0 - (-1) = +1 and coloured yellow on a node that can never validate: the sign inverted in exactly the bootstrap failure these signals exist to catch. Reports the disabled state as int64 max so headroom goes strongly negative instead. - The runbook claimed an expired list loads no keys. Expired counts as accepted, so its keys are loaded and then dropped by the expiry sweep, which calls for a different fix than replacing validators.txt. Pending is likewise a future-dated refresh, not a rejection. Documents both, plus how the quorum-disabled state now reads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -2122,9 +2122,14 @@ first one that is wrong and fix it before reading further panels.
|
||||
node already holds.
|
||||
- `fetch_error`, `bad_status`, `parse_error` — transport or content faults;
|
||||
the site is effectively unreachable.
|
||||
- `expired`, `stale`, `untrusted`, `invalid`, `unsupported_version`,
|
||||
`pending` — the list arrived but was rejected, so no keys are loaded from
|
||||
that site.
|
||||
- `stale`, `untrusted`, `invalid`, `unsupported_version` — the list arrived
|
||||
but was rejected, so no keys are loaded from that site.
|
||||
- `expired` — the list was applied and its keys were loaded, but it is past
|
||||
its validity window, so the expiry sweep drops them again and the
|
||||
publisher does not count as available. Refresh the publisher blob, and
|
||||
check the local clock, rather than replacing `validators.txt`.
|
||||
- `pending` — the list is valid only from a future date and is held for
|
||||
rotation. Normal, not a fault.
|
||||
Then read _UNL Trusted Keys vs Quorum_ and _UNL Quorum Headroom_
|
||||
(`unl_quorum`, `metric=trusted_keys` against `metric=quorum`). **This is
|
||||
the "will never validate" check.** If `trusted_keys` is zero, or sits at or
|
||||
@@ -2132,7 +2137,11 @@ first one that is wrong and fix it before reading further panels.
|
||||
too small to ever satisfy quorum: the node can track ledgers but will
|
||||
never declare one validated, and no amount of healthy acquire traffic
|
||||
changes that. A site stuck on `fetch_error` or `expired` in the panel above
|
||||
is the usual cause.
|
||||
is the usual cause. A very large `quorum` with a deeply negative headroom
|
||||
is the distinct "quorum disabled" state: too many publishers are
|
||||
unavailable, so quorum has been switched off entirely rather than merely
|
||||
set high. Fix publisher reachability first — the key count is irrelevant
|
||||
until quorum is enabled again.
|
||||
|
||||
5. **Clock — is local time disagreeing with the network?**
|
||||
Panel _Clock Close Offset_ (`clock_close_offset_seconds`, `metric=offset`).
|
||||
|
||||
@@ -42,7 +42,6 @@
|
||||
#include <stdexcept>
|
||||
#include <string>
|
||||
#include <string_view>
|
||||
#include <utility>
|
||||
|
||||
// VFALCO Shouldn't we have to include the OpenSSL
|
||||
// headers or something for SSL_get_finished?
|
||||
@@ -260,7 +259,7 @@ namespace {
|
||||
* @note Always throws; it never returns to its caller.
|
||||
*/
|
||||
[[noreturn]] void
|
||||
throwNegotiationFailure(Application& app, char const* reason, std::string message)
|
||||
throwNegotiationFailure(Application& app, char const* reason, std::string const& message)
|
||||
{
|
||||
XRPL_METRIC_COUNTER_INC_LABELED(
|
||||
app,
|
||||
@@ -268,7 +267,7 @@ throwNegotiationFailure(Application& app, char const* reason, std::string messag
|
||||
"Peer handshake negotiations rejected, by reason",
|
||||
{{"reason", std::string(reason)}});
|
||||
|
||||
throw std::runtime_error(std::move(message));
|
||||
throw std::runtime_error(message);
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
@@ -68,6 +68,7 @@
|
||||
#include <chrono>
|
||||
#include <cstddef>
|
||||
#include <cstdint>
|
||||
#include <limits>
|
||||
#include <memory>
|
||||
#include <sstream>
|
||||
#include <string>
|
||||
@@ -1491,7 +1492,19 @@ MetricsRegistry::registerUnlQuorumGauge()
|
||||
observe("trusted_keys", static_cast<int64_t>(validators.trustedKeyCount()));
|
||||
|
||||
// Validations required for a ledger to be fully validated.
|
||||
observe("quorum", static_cast<int64_t>(validators.quorum()));
|
||||
// ValidatorList disables quorum by returning SIZE_MAX when too
|
||||
// many publishers are unavailable. Casting that straight to
|
||||
// int64_t would wrap to -1 and make the headroom
|
||||
// (trusted_keys - quorum) read positive on a node that can
|
||||
// never validate, so report the disabled state as int64 max
|
||||
// instead: headroom then goes strongly negative, which is the
|
||||
// truthful signal.
|
||||
auto const quorum = validators.quorum();
|
||||
observe(
|
||||
"quorum",
|
||||
quorum == std::numeric_limits<std::size_t>::max()
|
||||
? std::numeric_limits<int64_t>::max()
|
||||
: static_cast<int64_t>(quorum));
|
||||
}
|
||||
catch (...) // NOLINT(bugprone-empty-catch)
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user