From 897b42354c62af01b08be53cf6e440eb5e1f5f3c Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 22 Jul 2026 19:15:55 -0400 Subject: [PATCH] Set the "rotation in flight" index right at the beginning of the rotation - Because the node copy process can take a long time, other ledgers may get validated. Any reads for those ledgers have the potential to be served by the archive DB and thus lost, too. Why wait? - Also added an assertion suggested on @vlntb in #7763. --- include/xrpl/nodestore/DatabaseRotating.h | 12 +++--- .../nodestore/detail/DatabaseRotatingImp.h | 2 + src/libxrpl/nodestore/DatabaseRotatingImp.cpp | 8 +++- src/xrpld/app/misc/SHAMapStoreImp.cpp | 43 +++++++++++-------- 4 files changed, 39 insertions(+), 26 deletions(-) diff --git a/include/xrpl/nodestore/DatabaseRotating.h b/include/xrpl/nodestore/DatabaseRotating.h index 9be1c399ec..5872e553d6 100644 --- a/include/xrpl/nodestore/DatabaseRotating.h +++ b/include/xrpl/nodestore/DatabaseRotating.h @@ -46,15 +46,15 @@ public: /** * 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. + * While in flight, a read for ledgers after the inFlight value 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(LedgerIndex inFlight) = 0; + virtual LedgerIndex + getRotationInFlight() const = 0; }; } // namespace xrpl::NodeStore diff --git a/include/xrpl/nodestore/detail/DatabaseRotatingImp.h b/include/xrpl/nodestore/detail/DatabaseRotatingImp.h index 36737a23a9..2bec58c674 100644 --- a/include/xrpl/nodestore/detail/DatabaseRotatingImp.h +++ b/include/xrpl/nodestore/detail/DatabaseRotatingImp.h @@ -73,6 +73,8 @@ public: void setRotationInFlight(LedgerIndex inFlight) override; + LedgerIndex + getRotationInFlight() const override; private: std::shared_ptr writableBackend_; diff --git a/src/libxrpl/nodestore/DatabaseRotatingImp.cpp b/src/libxrpl/nodestore/DatabaseRotatingImp.cpp index c6998021bf..68cf0d0949 100644 --- a/src/libxrpl/nodestore/DatabaseRotatingImp.cpp +++ b/src/libxrpl/nodestore/DatabaseRotatingImp.cpp @@ -89,6 +89,12 @@ DatabaseRotatingImp::setRotationInFlight(LedgerIndex inFlight) JLOG(j_.debug()) << "Rotating: copy-forward on archive reads from " << inFlight << " forward"; } +LedgerIndex +DatabaseRotatingImp::getRotationInFlight() const +{ + return rotationInFlight_.load(std::memory_order_acquire); +} + std::string DatabaseRotatingImp::getName() const { @@ -199,7 +205,7 @@ DatabaseRotatingImp::fetchNodeObject( // 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. - auto const inFlight = rotationInFlight_.load(std::memory_order_acquire); + auto const inFlight = getRotationInFlight(); if (duplicate || (inFlight != 0 && (ledgerSeq == 0 || ledgerSeq >= inFlight))) { { diff --git a/src/xrpld/app/misc/SHAMapStoreImp.cpp b/src/xrpld/app/misc/SHAMapStoreImp.cpp index afd7eb9553..aec5d503f5 100644 --- a/src/xrpld/app/misc/SHAMapStoreImp.cpp +++ b/src/xrpld/app/misc/SHAMapStoreImp.cpp @@ -313,6 +313,11 @@ SHAMapStoreImp::run() while (true) { + XRPL_ASSERT( + dbRotating_->getRotationInFlight() == 0, + "SHAMapStoreImp::run : rotationInFlight_ must be zero " + "outside rotation window"); + healthy_ = true; std::shared_ptr validatedLedger; @@ -366,6 +371,25 @@ SHAMapStoreImp::run() << ledgerMaster_->getValidatedLedgerAge().count() << "s. Complete ledgers: " << ledgerMaster_->getCompleteLedgers(); + // Close the getKeys()->swap exposure window: from here until + // rotate() completes, an ordinary read for new ledgers 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(0); + } + }; + RotationExposureGuard const rotationExposureGuard{*dbRotating_}; + // Anything before lastRotated is going to get deleted soon, so we don't care about + // moving it to the writable DB. + dbRotating_->setRotationInFlight(lastRotated); + clearPrior(lastRotated); if (healthWait() == HealthResult::Stopping) return; @@ -393,25 +417,6 @@ 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(0); - } - }; - RotationExposureGuard const rotationExposureGuard{*dbRotating_}; - // Anything before lastRotated is going to get deleted soon, so we don't care about - // moving it to the writable DB. - dbRotating_->setRotationInFlight(lastRotated); - JLOG(journal_.debug()) << "freshening caches"; freshenCaches(); if (healthWait() == HealthResult::Stopping)