From 56325f56c2533cbe5fa8bd21954dbae4b852a349 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Tue, 28 Jul 2026 18:25:22 +0100 Subject: [PATCH] feat(ledger): attribute deferrals and timeouts to ledger acquisition Both counters are recorded in TimeoutCounter, a base shared by five subclasses with different job limits, so they pooled every lane together. The documented fingerprint for a stalled acquisition is deferrals rising while timeouts stay flat -- but a saturated replay lane reproduces that shape while ledger acquisition is healthy, and the two counters being compared were not drawn from the same population. Adds ledger-acquisition-scoped counts alongside the totals, discriminated by the job name already held at both call sites, so no new state and no signature change. The all-lane counters stay for callers that want them. Their exporters went in with the preceding commit, which shared a file. Co-Authored-By: Claude Opus 5 (1M context) --- src/xrpld/app/ledger/AcquireStats.h | 44 ++++++++++++++++++- .../app/ledger/detail/TimeoutCounter.cpp | 4 +- src/xrpld/app/ledger/detail/TimeoutCounter.h | 17 +++++++ 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/src/xrpld/app/ledger/AcquireStats.h b/src/xrpld/app/ledger/AcquireStats.h index 027a6b346b..7ed93e85c7 100644 --- a/src/xrpld/app/ledger/AcquireStats.h +++ b/src/xrpld/app/ledger/AcquireStats.h @@ -93,9 +93,11 @@ public: * advance the retry count toward give-up. */ void - recordDeferral() + recordDeferral(bool ledgerAcquisition = false) { deferrals_.fetch_add(1, std::memory_order_relaxed); + if (ledgerAcquisition) + ledgerDeferrals_.fetch_add(1, std::memory_order_relaxed); } /** @@ -105,9 +107,11 @@ public: * separate from deferrals. */ void - recordTimeout() + recordTimeout(bool ledgerAcquisition = false) { timeouts_.fetch_add(1, std::memory_order_relaxed); + if (ledgerAcquisition) + ledgerTimeouts_.fetch_add(1, std::memory_order_relaxed); } /** @@ -171,6 +175,32 @@ public: return timeouts_.load(std::memory_order_relaxed); } + /** + * Deferrals that belong to ledger acquisition only. + * + * @ref getDeferrals covers every TimeoutCounter subclass, so a busy + * replay or transaction-set lane inflates it. Compare this against + * @ref getLedgerTimeouts to judge ledger acquisition on its own. + */ + [[nodiscard]] std::uint64_t + getLedgerDeferrals() const + { + return ledgerDeferrals_.load(std::memory_order_relaxed); + } + + /** + * Timeouts that belong to ledger acquisition only. + * + * The partner of @ref getLedgerDeferrals: rising deferrals with flat + * timeouts here means ledger acquisition's give-up path is disarmed, + * which the all-lane counters cannot show. + */ + [[nodiscard]] std::uint64_t + getLedgerTimeouts() const + { + return ledgerTimeouts_.load(std::memory_order_relaxed); + } + /** * Return the number of acquisitions that exhausted their retry budget. */ @@ -224,6 +254,16 @@ private: */ std::atomic deferrals_{0}; + /** + * Deferrals attributable to ledger acquisition alone. + */ + std::atomic ledgerDeferrals_{0}; + + /** + * Timeouts attributable to ledger acquisition alone. + */ + std::atomic ledgerTimeouts_{0}; + /** * Timer bodies that ran and advanced the retry count. */ diff --git a/src/xrpld/app/ledger/detail/TimeoutCounter.cpp b/src/xrpld/app/ledger/detail/TimeoutCounter.cpp index c5a54b8cc9..3e9961bfb1 100644 --- a/src/xrpld/app/ledger/detail/TimeoutCounter.cpp +++ b/src/xrpld/app/ledger/detail/TimeoutCounter.cpp @@ -67,7 +67,7 @@ TimeoutCounter::queueJob(ScopedLockType& sl) // Counted separately from timeouts: this path re-arms the timer // without running invokeOnTimer, so timeouts_ does not advance and the // give-up test that reads it cannot fire while the lane stays full. - app_.getAcquireStats().recordDeferral(); + app_.getAcquireStats().recordDeferral(isLedgerAcquisition()); JLOG(journal_.debug()) << "Deferring " << queueJobParameter_.jobName << " timer due to load"; setTimer(sl); @@ -92,7 +92,7 @@ TimeoutCounter::invokeOnTimer() if (!progress_) { ++timeouts_; - app_.getAcquireStats().recordTimeout(); + app_.getAcquireStats().recordTimeout(isLedgerAcquisition()); JLOG(journal_.debug()) << "Timeout(" << timeouts_ << ") " << " acquiring " << hash_; onTimer(false, sl); diff --git a/src/xrpld/app/ledger/detail/TimeoutCounter.h b/src/xrpld/app/ledger/detail/TimeoutCounter.h index 682abf1537..bba22139be 100644 --- a/src/xrpld/app/ledger/detail/TimeoutCounter.h +++ b/src/xrpld/app/ledger/detail/TimeoutCounter.h @@ -139,6 +139,23 @@ protected: QueueJobParameter queueJobParameter_; + /** + * Whether this counter belongs to ledger acquisition. + * + * Deferrals and timeouts are recorded in this base class, so they would + * otherwise pool every subclass together: a saturated replay lane would + * look exactly like a stalled ledger acquisition. The job name already + * identifies the subclass, so it is the cheapest discriminator available + * and needs no extra state. + * + * @return True for the InboundLedger lane, false for every other. + */ + [[nodiscard]] bool + isLedgerAcquisition() const + { + return queueJobParameter_.jobName == "InboundLedger"; + } + private: /** * Calls onTimer() if in the right state.