From 47da8cccd6d64adfef4e1bf3c3a9067e318a25ee Mon Sep 17 00:00:00 2001 From: Nicholas Dudfield Date: Tue, 14 Apr 2026 17:00:49 +0700 Subject: [PATCH] fix: dispatch retired-ledger destruction off advance thread unconditionally Previously, when shouldRetire was false (pre-TRACKING, before the sticky caught-up flag flipped), retiredLedgers fell out of scope at the end of setFullLedger and destructed synchronously on the advance thread. Each publish past ledger_history cascaded through a million-leaf destruction before doAdvance could loop to the next publishable ledger, producing a stall-then-flurry pattern during catch-up. Always move retiredLedgers into the async job. Inside the job, the shouldRetire capture gates only the bookkeeping side effects (mCompleteLedgers / relational / LedgerHistory pruning). Destruction of the captured shared_ptrs happens on the worker regardless, so the advance thread stays on the publish hot path. --- src/xrpld/app/ledger/detail/LedgerMaster.cpp | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/xrpld/app/ledger/detail/LedgerMaster.cpp b/src/xrpld/app/ledger/detail/LedgerMaster.cpp index 66277a6cd..13d5a8858 100644 --- a/src/xrpld/app/ledger/detail/LedgerMaster.cpp +++ b/src/xrpld/app/ledger/detail/LedgerMaster.cpp @@ -984,13 +984,27 @@ LedgerMaster::setFullLedger( // Ledgers' SHAMap spines). The retired Ledgers stay alive in the // captured vector until the job runs; destruction happens on the // worker thread, off doAdvance's critical path. - if (shouldRetire && !retiredLedgers.empty()) + // + // Dispatch unconditionally whenever we have retired Ledgers — even + // pre-TRACKING, where shouldRetire is false and we skip the + // mCompleteLedgers / relational / LedgerHistory pruning. The job + // still owns the shared_ptrs, so their destruction cascade runs on + // the worker, not on the advance thread. Without this, retired + // Ledgers fall out of scope synchronously in setFullLedger and the + // advance thread blocks on a million-leaf destruction per publish, + // producing the sync-stall-then-flurry pattern during catch-up. + if (!retiredLedgers.empty()) { app_.getJobQueue().addJob( jtLEDGER_DATA, "retireLedgers", - [&app = app_, retired = std::move(retiredLedgers)]() { - app.getSHAMapStore().retireLedgers(retired); + [&app = app_, shouldRetire, retired = std::move(retiredLedgers)]() { + if (shouldRetire) + app.getSHAMapStore().retireLedgers(retired); + // Otherwise `retired` just destructs here on this + // worker thread as the lambda exits — bookkeeping + // side effects skipped, destruction cascade kept off + // the advance thread either way. }); }