Limit copy-forward to ledgers we're not about to delete, and unknown

include/xrpl/nodestore/DatabaseRotating.h

@@ -4,6 +4,7 @@
 #include <xrpl/nodestore/Backend.h>
 #include <xrpl/nodestore/Database.h>
 #include <xrpl/nodestore/Scheduler.h>
+#include <xrpl/protocol/Protocol.h>
 
 #include <functional>
 #include <memory>
@@ -53,7 +54,7 @@ public:
      * dropped.
      */
     virtual void
-    setRotationInFlight(bool inFlight) = 0;
+    setRotationInFlight(LedgerIndex inFlight) = 0;
 };
 
 }  // namespace xrpl::NodeStore

include/xrpl/nodestore/detail/DatabaseRotatingImp.h

@@ -8,6 +8,7 @@
 #include <xrpl/nodestore/DatabaseRotating.h>
 #include <xrpl/nodestore/NodeObject.h>
 #include <xrpl/nodestore/Scheduler.h>
+#include <xrpl/protocol/Protocol.h>
 
 #include <atomic>
 #include <cstdint>
@@ -71,20 +72,25 @@ public:
     sweep() override;
 
     void
-    setRotationInFlight(bool inFlight) override;
+    setRotationInFlight(LedgerIndex 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
+    // Set to the index of the last rotated ledger between SHAMapStore
+    // starting the cache-freshen phase and the completion of rotate().
+    // While non-zero, archive hits on ordinary (duplicate == false)
+    // fetches are copied forward into the writable backend if they are
+    // for that ledger or later, since those are the ones we'll keep.
+    // To be safe, copy forward if the provided ledger index is 0.
+    // copyForwardCount_ tallies them per rotation for the
     // summary line logged at swap.
-    std::atomic<bool> rotationInFlight_{false};
+    // copyRejectCount_ tallies the ones that weren't copied.
+    std::atomic<LedgerIndex> rotationInFlight_{0};
     std::atomic<std::uint64_t> copyForwardCount_{0};
+    std::atomic<std::uint64_t> copyRejectCount_{0};
 
     std::shared_ptr<NodeObject>
     fetchNodeObject(uint256 const& hash, std::uint32_t, FetchReport& fetchReport, bool duplicate)

src/libxrpl/nodestore/DatabaseRotatingImp.cpp

@@ -12,6 +12,7 @@
 #include <xrpl/nodestore/NodeObject.h>
 #include <xrpl/nodestore/Scheduler.h>
 #include <xrpl/nodestore/Types.h>
+#include <xrpl/protocol/Protocol.h>
 
 #include <atomic>
 #include <cstdint>
@@ -54,6 +55,7 @@ DatabaseRotatingImp::rotate(
     // deleted.
     std::shared_ptr<NodeStore::Backend> oldArchiveBackend;
     std::uint64_t copyForwards = 0;
+    std::uint64_t copyRejects = 0;
     {
         std::scoped_lock const lock(mutex_);
 
@@ -66,24 +68,25 @@ DatabaseRotatingImp::rotate(
         writableBackend_ = std::move(newBackend);
 
         copyForwards = copyForwardCount_.exchange(0, std::memory_order_relaxed);
+        copyRejects = copyRejectCount_.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";
+                           "during the rotation window. Rejected "
+                        << copyRejects;
     }
 
     f(newWritableBackendName, newArchiveBackendName);
 }
 
 void
-DatabaseRotatingImp::setRotationInFlight(bool inFlight)
+DatabaseRotatingImp::setRotationInFlight(LedgerIndex inFlight)
 {
     rotationInFlight_.store(inFlight, std::memory_order_release);
-    JLOG(j_.debug()) << "Rotating: copy-forward on archive reads "
-                     << (inFlight ? "enabled" : "disabled");
+    JLOG(j_.debug()) << "Rotating: copy-forward on archive reads from " << inFlight << " forward";
 }
 
 std::string
@@ -196,7 +199,8 @@ 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.
-            if (duplicate || rotationInFlight_.load(std::memory_order_acquire))
+            auto const inFlight = rotationInFlight_.load(std::memory_order_acquire);
+            if (duplicate || (inFlight != 0 && (ledgerSeq == 0 || ledgerSeq >= inFlight)))
             {
                 {
                     // Refresh the writable backend pointer
@@ -212,6 +216,12 @@ DatabaseRotatingImp::fetchNodeObject(
                 }
                 writable->store(nodeObject);
             }
+            else if (inFlight != 0)
+            {
+                JLOG(j_.warn()) << "Rotating: DO NOT copy node for ledger " << ledgerSeq
+                                << " from archive to writable backend: " << hash;
+                copyRejectCount_.fetch_add(1, std::memory_order_relaxed);
+            }
         }
     }
 

src/xrpld/app/misc/SHAMapStoreImp.cpp

@@ -404,11 +404,11 @@ SHAMapStoreImp::run()
                 NodeStore::DatabaseRotating& db;
                 ~RotationExposureGuard()
                 {
-                    db.setRotationInFlight(false);
+                    db.setRotationInFlight(0);
                 }
             };
             RotationExposureGuard const rotationExposureGuard{*dbRotating_};
-            dbRotating_->setRotationInFlight(true);
+            dbRotating_->setRotationInFlight(lastRotated);
 
             JLOG(journal_.debug()) << "freshening caches";
             freshenCaches();