Merge branch 'develop' into ximinez/fix-getkeys

include/xrpl/nodestore/DatabaseRotating.h

@@ -41,6 +41,19 @@ public:
         std::unique_ptr<NodeStore::Backend>&& newBackend,
         std::function<void(std::string const& writableName, std::string const& archiveName)> 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

include/xrpl/nodestore/detail/DatabaseRotatingImp.h

@@ -9,6 +9,7 @@
 #include <xrpl/nodestore/NodeObject.h>
 #include <xrpl/nodestore/Scheduler.h>
 
+#include <atomic>
 #include <cstdint>
 #include <functional>
 #include <memory>
@@ -69,11 +70,22 @@ public:
     void
     sweep() override;
 
+    void
+    setRotationInFlight(bool inFlight) override;
+
 private:
     std::shared_ptr<Backend> writableBackend_;
     std::shared_ptr<Backend> 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<bool> rotationInFlight_{false};
+    std::atomic<std::uint64_t> copyForwardCount_{0};
+
     std::shared_ptr<NodeObject>
     fetchNodeObject(uint256 const& hash, std::uint32_t, FetchReport& fetchReport, bool duplicate)
         override;

src/libxrpl/nodestore/DatabaseRotatingImp.cpp

@@ -13,6 +13,7 @@
 #include <xrpl/nodestore/Scheduler.h>
 #include <xrpl/nodestore/Types.h>
 
+#include <atomic>
 #include <cstdint>
 #include <exception>
 #include <functional>
@@ -52,6 +53,7 @@ DatabaseRotatingImp::rotate(
     // callback finishes. Only then will the archive directory be
     // deleted.
     std::shared_ptr<NodeStore::Backend> 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);
+            }
         }
     }
 

src/xrpld/app/misc/SHAMapStoreImp.cpp

@@ -16,9 +16,11 @@
 #include <xrpl/ledger/Ledger.h>
 #include <xrpl/nodestore/Database.h>
 #include <xrpl/nodestore/Manager.h>
+#include <xrpl/nodestore/NodeObject.h>
 #include <xrpl/nodestore/Scheduler.h>
 #include <xrpl/nodestore/detail/DatabaseRotatingImp.h>
 #include <xrpl/protocol/Protocol.h>
+#include <xrpl/protocol/Serializer.h>
 #include <xrpl/server/NetworkOPs.h>
 #include <xrpl/server/State.h>
 #include <xrpl/shamap/SHAMapMissingNode.h>
@@ -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<int>(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)