From ae3898d8b406d5d9a19f88094f6985620aba7b86 Mon Sep 17 00:00:00 2001 From: Sophia Xie Date: Sat, 11 Jul 2026 16:30:04 -0700 Subject: [PATCH] fix: Copy archive reads forward during rotation to prevent node loss --- include/xrpl/nodestore/DatabaseRotating.h | 12 +++++++ .../nodestore/detail/DatabaseRotatingImp.h | 12 +++++++ src/libxrpl/nodestore/DatabaseRotatingImp.cpp | 31 +++++++++++++++++-- src/xrpld/app/misc/SHAMapStoreImp.cpp | 17 ++++++++++ 4 files changed, 70 insertions(+), 2 deletions(-) diff --git a/include/xrpl/nodestore/DatabaseRotating.h b/include/xrpl/nodestore/DatabaseRotating.h index 69eb31261d..99f395d10b 100644 --- a/include/xrpl/nodestore/DatabaseRotating.h +++ b/include/xrpl/nodestore/DatabaseRotating.h @@ -40,6 +40,18 @@ 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..4b5d1ee2d8 100644 --- a/src/libxrpl/nodestore/DatabaseRotatingImp.cpp +++ b/src/libxrpl/nodestore/DatabaseRotatingImp.cpp @@ -52,6 +52,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 +63,28 @@ DatabaseRotatingImp::rotate( newArchiveBackendName = archiveBackend_->getName(); writableBackend_ = std::move(newBackend); + + copyForwards = copyForwardCount_.exchange(0, std::memory_order_acq_rel); + } + + 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 +195,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_; writable->store(nodeObject); + } } } diff --git a/src/xrpld/app/misc/SHAMapStoreImp.cpp b/src/xrpld/app/misc/SHAMapStoreImp.cpp index 643c0b18a7..ec6210705d 100644 --- a/src/xrpld/app/misc/SHAMapStoreImp.cpp +++ b/src/xrpld/app/misc/SHAMapStoreImp.cpp @@ -365,6 +365,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)