From a0fd1cce542f2ab9e43b9d9857d5173927966b50 Mon Sep 17 00:00:00 2001 From: Sophia Xie <106177003+sophiax851@users.noreply.github.com> Date: Tue, 14 Jul 2026 16:42:40 -0700 Subject: [PATCH 1/3] fix: Re-store nodes missing from both backends during online_delete rotation (#7763) Co-authored-by: Valentin Balaschenko <13349202+vlntb@users.noreply.github.com> --- include/xrpl/nodestore/DatabaseRotating.h | 13 +++++++ .../nodestore/detail/DatabaseRotatingImp.h | 12 ++++++ src/libxrpl/nodestore/DatabaseRotatingImp.cpp | 32 +++++++++++++++- src/xrpld/app/misc/SHAMapStoreImp.cpp | 37 ++++++++++++++++++- 4 files changed, 91 insertions(+), 3 deletions(-) diff --git a/include/xrpl/nodestore/DatabaseRotating.h b/include/xrpl/nodestore/DatabaseRotating.h index 1c5bb0efaf..5381b5c435 100644 --- a/include/xrpl/nodestore/DatabaseRotating.h +++ b/include/xrpl/nodestore/DatabaseRotating.h @@ -41,6 +41,19 @@ public: std::unique_ptr&& newBackend, std::function const& f) = 0; + + /** + * Marks an online-delete rotation as in progress (or completed). + * + * While in flight, a read served by the archive backend is copied + * forward into the writable backend even for ordinary + * (duplicate == false) fetches: the archive is about to be deleted, + * and a node body canonicalized into caches during the rotation + * window would otherwise survive only in RAM once the archive is + * dropped. + */ + virtual void + setRotationInFlight(bool inFlight) = 0; }; } // namespace xrpl::NodeStore diff --git a/include/xrpl/nodestore/detail/DatabaseRotatingImp.h b/include/xrpl/nodestore/detail/DatabaseRotatingImp.h index 6343275c76..ecbe9a513d 100644 --- a/include/xrpl/nodestore/detail/DatabaseRotatingImp.h +++ b/include/xrpl/nodestore/detail/DatabaseRotatingImp.h @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -69,11 +70,22 @@ public: void sweep() override; + void + setRotationInFlight(bool inFlight) override; + private: std::shared_ptr writableBackend_; std::shared_ptr archiveBackend_; mutable std::mutex mutex_; + // True between SHAMapStore starting the cache-freshen phase and the + // completion of rotate(). While true, archive hits on ordinary + // (duplicate == false) fetches are copied forward into the writable + // backend; copyForwardCount_ tallies them per rotation for the + // summary line logged at swap. + std::atomic rotationInFlight_{false}; + std::atomic copyForwardCount_{0}; + std::shared_ptr fetchNodeObject(uint256 const& hash, std::uint32_t, FetchReport& fetchReport, bool duplicate) override; diff --git a/src/libxrpl/nodestore/DatabaseRotatingImp.cpp b/src/libxrpl/nodestore/DatabaseRotatingImp.cpp index 7f4dca3ed1..23a48a3bf3 100644 --- a/src/libxrpl/nodestore/DatabaseRotatingImp.cpp +++ b/src/libxrpl/nodestore/DatabaseRotatingImp.cpp @@ -13,6 +13,7 @@ #include #include +#include #include #include #include @@ -52,6 +53,7 @@ DatabaseRotatingImp::rotate( // callback finishes. Only then will the archive directory be // deleted. std::shared_ptr oldArchiveBackend; + std::uint64_t copyForwards = 0; { std::scoped_lock const lock(mutex_); @@ -62,11 +64,28 @@ DatabaseRotatingImp::rotate( newArchiveBackendName = archiveBackend_->getName(); writableBackend_ = std::move(newBackend); + + copyForwards = copyForwardCount_.exchange(0, std::memory_order_relaxed); + } + + if (copyForwards > 0) + { + JLOG(j_.warn()) << "Rotating: copied forward " << copyForwards + << " archive-served reads into the writable backend " + "during the rotation window"; } f(newWritableBackendName, newArchiveBackendName); } +void +DatabaseRotatingImp::setRotationInFlight(bool inFlight) +{ + rotationInFlight_.store(inFlight, std::memory_order_release); + JLOG(j_.debug()) << "Rotating: copy-forward on archive reads " + << (inFlight ? "enabled" : "disabled"); +} + std::string DatabaseRotatingImp::getName() const { @@ -177,9 +196,18 @@ DatabaseRotatingImp::fetchNodeObject( writable = writableBackend_; } - // Update writable backend with data from the archive backend - if (duplicate) + // Update writable backend with data from the archive backend. + // While a rotation is in flight, ordinary (duplicate == false) + // reads served by the archive are copied forward too: the + // archive is about to be deleted, and a body canonicalized + // into the cache after the freshen getKeys() snapshot would + // otherwise survive only in RAM once the archive is dropped. + if (duplicate || rotationInFlight_.load(std::memory_order_acquire)) + { + if (!duplicate) + copyForwardCount_.fetch_add(1, std::memory_order_relaxed); writable->store(nodeObject); + } } } diff --git a/src/xrpld/app/misc/SHAMapStoreImp.cpp b/src/xrpld/app/misc/SHAMapStoreImp.cpp index 6167b5d772..9b5f412fc5 100644 --- a/src/xrpld/app/misc/SHAMapStoreImp.cpp +++ b/src/xrpld/app/misc/SHAMapStoreImp.cpp @@ -16,9 +16,11 @@ #include #include #include +#include #include #include #include +#include #include #include #include @@ -254,8 +256,24 @@ bool SHAMapStoreImp::copyNode(std::uint64_t& nodeCount, SHAMapTreeNode const& node) { // Copy a single record from node to dbRotating_ - dbRotating_->fetchNodeObject( + auto obj = dbRotating_->fetchNodeObject( node.getHash().asUInt256(), 0, NodeStore::FetchType::Synchronous, true); + if (!obj) + { + XRPL_ASSERT(node.cowid() == 0, "SHAMapStoreImp::copyNode : rescued node must be clean"); + // Reachable from the validated state map in memory, but present in + // neither backend: its only on-disk copy lived in a backend removed by + // an earlier rotation, and it was never rewritten because it is clean + // (cowid == 0, so flushDirty skips it). Persist the in-memory body + // directly into the writable backend so it survives this rotation + // instead of later surfacing as an unresolvable SHAMapMissingNode. + auto const hash = node.getHash().asUInt256(); + Serializer s; + node.serializeWithPrefix(s); + dbRotating_->store(NodeObjectType::AccountNode, std::move(s.modData()), hash, 0); + JLOG(journal_.warn()) << "copyNode: re-stored node missing from both backends, hash=" + << hash << " type=" << static_cast(node.getType()); + } if ((++nodeCount % checkHealthInterval_) == 0u) { if (healthWait() == HealthResult::Stopping) @@ -348,6 +366,23 @@ SHAMapStoreImp::run() JLOG(journal_.debug()) << "copied ledger " << validatedSeq << " nodecount " << nodeCount; + // Close the getKeys()->swap exposure window: from here until + // rotate() completes, an ordinary read served by the archive is + // copied forward into the writable backend, so a node fetched + // from the doomed archive cannot be left RAM-only when the + // archive is deleted. RAII so the early returns below (and any + // exception) also clear the flag. + struct RotationExposureGuard + { + NodeStore::DatabaseRotating& db; + ~RotationExposureGuard() + { + db.setRotationInFlight(false); + } + }; + RotationExposureGuard const rotationExposureGuard{*dbRotating_}; + dbRotating_->setRotationInFlight(true); + JLOG(journal_.debug()) << "freshening caches"; freshenCaches(); if (healthWait() == HealthResult::Stopping) From a24e543af3b1aaa25803835bae54df2b1a743349 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 15 Jul 2026 09:30:20 -0400 Subject: [PATCH 2/3] fix: Allocate TaggedCache::getKeys() memory outside of lock (#7567) Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com> --- include/xrpl/basics/TaggedCache.ipp | 41 +++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/include/xrpl/basics/TaggedCache.ipp b/include/xrpl/basics/TaggedCache.ipp index b79561c71a..447743a7b7 100644 --- a/include/xrpl/basics/TaggedCache.ipp +++ b/include/xrpl/basics/TaggedCache.ipp @@ -3,6 +3,9 @@ #include #include // IWYU pragma: keep #include +#include + +#include namespace xrpl { @@ -601,8 +604,42 @@ TaggedCache v; { - std::scoped_lock const lock(mutex_); - v.reserve(cache_.size()); + // Keep track of how many iterations are needed. Exit the loop if the number of retries gets + // absurd. (Note that if this somehow ever happens, one more allocation will be done under + // lock, which is undesirable, but really should be almost impossible.) + std::size_t allocationIterations = 0; + std::unique_lock lock(mutex_); + for (auto size = cache_.size(); v.capacity() < size && allocationIterations < 20; + size = cache_.size()) + { + ScopeUnlock const unlock(lock); + if (allocationIterations > 0) + { + JLOG(journal_.info()) + << "getKeys(): Cache grew beyond allocated capacity after " + << allocationIterations << " prior attempt(s). Have " << v.capacity() + << ", need " << size << ". Retrying allocation"; + } + // Allocate the current size plus a little extra, in case the cache grows while + // allocating. Each time another allocation is needed, the extra also gets bigger until + // it ultimately doubles the size + 1. + constexpr std::size_t baseShift = 5; + auto const bufferOffset = std::min(allocationIterations, std::size_t{baseShift}); + auto const bufferShift = baseShift - bufferOffset; + size += (size >> bufferShift) + 1; + v.reserve(size); + ++allocationIterations; + } + if (v.capacity() < cache_.size()) + { + // LCOV_EXCL_START + UNREACHABLE("xrpl::TaggedCache::getKeys(): failed to allocate sufficient capacity"); + v.reserve(cache_.size()); + // LCOV_EXCL_STOP + } + XRPL_ASSERT(lock.owns_lock(), "xrpl::TaggedCache::getKeys(): owns lock"); + XRPL_ASSERT( + v.capacity() >= cache_.size(), "xrpl::TaggedCache::getKeys(): sufficient capacity"); for (auto const& _ : cache_) v.push_back(_.first); } From 781ab723afb1a55748405f91447a40d69c5442f7 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Wed, 15 Jul 2026 19:24:31 +0100 Subject: [PATCH 3/3] ci: Fix workflow launch on matrix-unrelated labels (#7812) --- .github/workflows/on-pr.yml | 37 +++++++++++++------------------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/.github/workflows/on-pr.yml b/.github/workflows/on-pr.yml index bbf6f8c39e..442a202a44 100644 --- a/.github/workflows/on-pr.yml +++ b/.github/workflows/on-pr.yml @@ -25,18 +25,11 @@ on: - unlabeled concurrency: - # Use a per-ref group so a newer run (a push, or a change to a label below) - # supersedes the in-progress one for that ref. Label events we don't act on get - # their own unique group (per run id) instead, keeping them out of the shared - # group so real builds keep running. Keep this list in sync with `should-run`. - group: >- - ${{ github.workflow }}-${{ github.ref }}${{ - ((github.event.action == 'labeled' || github.event.action == 'unlabeled') - && github.event.label.name != 'Ready to merge' - && github.event.label.name != 'DraftRunCI' - && github.event.label.name != 'Full CI build') - && format('-{0}', github.run_id) || '' - }} + # A single per-ref group with cancel-in-progress means any newer run (a push + # or a label change) supersedes the in-progress one for that ref. Keeping + # exactly one authoritative run per ref ensures a fast do-nothing run can never + # mask a real build's checks. + group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true defaults: @@ -44,21 +37,17 @@ defaults: shell: bash jobs: - # This job determines whether the rest of the workflow should run. It runs - # when the PR is not a draft (which should also cover merge-group) or has the - # 'DraftRunCI' or 'Full CI build' label. For label events it only runs when the - # label added or removed is one we act on ('Ready to merge', 'DraftRunCI' or - # 'Full CI build'), so unrelated label changes do not trigger a redundant run. + # This job determines whether the rest of the workflow should run at all, + # based on the current set of labels: it runs when the PR is not a draft + # (which should also cover merge-group) or has the 'DraftRunCI' or + # 'Full CI build' label. Whether a build then happens, and whether it is the + # minimal or full matrix, is decided further below and in the strategy matrix. should-run: if: >- ${{ - ((github.event.action != 'labeled' && github.event.action != 'unlabeled') - || github.event.label.name == 'Ready to merge' - || github.event.label.name == 'DraftRunCI' - || github.event.label.name == 'Full CI build') - && (!github.event.pull_request.draft - || contains(github.event.pull_request.labels.*.name, 'DraftRunCI') - || contains(github.event.pull_request.labels.*.name, 'Full CI build')) + !github.event.pull_request.draft + || contains(github.event.pull_request.labels.*.name, 'DraftRunCI') + || contains(github.event.pull_request.labels.*.name, 'Full CI build') }} runs-on: ubuntu-latest steps: