From ae3e4aa56b3d93a46101bf27b3713d70b1d70aa7 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 8 Jul 2025 19:12:13 -0400 Subject: [PATCH] Create a unit test demonstrating "lost" ledgers - It is possible for changed ledger objects to be written to the ArchiveDB during the online delete rotation process before the writable DB is created. When the rotation finishes, `lastRotated` is updated to the ledger sequence from when the process started, on the assumption that all ledgers after that will be written to the new writable DB, but some of them are in the old DB. Next time rotation runs, the data for those old ledgers is not brought over, and thus is deleted from the Archive DB. --- src/test/app/SHAMapStore_test.cpp | 35 ++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/src/test/app/SHAMapStore_test.cpp b/src/test/app/SHAMapStore_test.cpp index 47e4750411..743397fae9 100644 --- a/src/test/app/SHAMapStore_test.cpp +++ b/src/test/app/SHAMapStore_test.cpp @@ -662,6 +662,7 @@ public: auto const alice = Account("alice"); env.fund(XRP(1000), alice); env.close(); + storeHash(); auto& lm = env.app().getLedgerMaster(); LedgerIndex minSeq = 2; @@ -669,6 +670,8 @@ public: auto& store = env.app().getSHAMapStore(); store.rendezvous(); LedgerIndex lastRotated = store.getLastRotated(); + LedgerIndex betterMinSeq = minSeq; + LedgerIndex betterLastRotated = lastRotated; BEAST_EXPECTS(maxSeq == 3, to_string(maxSeq)); BEAST_EXPECTS( lm.getCompleteLedgers() == "2-3", lm.getCompleteLedgers()); @@ -684,13 +687,14 @@ public: lm.missingFromCompleteLedgerRange(minSeq + 2, maxSeq + 2) == 2); // Close enough ledgers to rotate a few times - while (maxSeq < 20) + while (maxSeq < 28) { for (int t = 0; t < 3; ++t) { env(noop(alice)); } env.close(); + storeHash(); store.rendezvous(); ++maxSeq; @@ -733,6 +737,7 @@ public: // Close another ledger, which will trigger a rotation, but the // rotation will be stuck until the missing ledger is filled in. env.close(); + storeHash(); // DO NOT CALL rendezvous()! You'll end up with a deadlock. ++maxSeq; @@ -757,6 +762,7 @@ public: for (int l = 0; l < 5; ++l) { env.close(); + storeHash(); // DO NOT CALL rendezvous()! You'll end up with a deadlock. ++maxSeq; // Nothing has changed @@ -784,6 +790,33 @@ public: minSeq = lastRotated; lastRotated = deleteSeq + 1; + betterMinSeq = betterLastRotated; + betterLastRotated = maxSeq + 2; + + // Bypass caching and try to load the ledger roots from the node + // store + for (auto const& [seq, hash] : hashes) + { + auto const nodeObject = ns.fetchNodeObject(hash); + std::stringstream ss; + ss << "minSeq: " << minSeq << ", maxSeq: " << maxSeq + << ", search: " << seq; + if (betterMinSeq != minSeq) + ss << ", betterMinSeq: " << betterMinSeq; + ss << ". Should " << (seq < minSeq ? "NOT " : "") + << "be found"; + BEAST_EXPECT(minSeq <= betterMinSeq); + if (betterMinSeq != minSeq && seq >= minSeq && + seq < betterMinSeq) + ss << ", but actually will NOT be found"; + if (seq < minSeq) + BEAST_EXPECTS(!nodeObject, ss.str()); + // Uncommenting this condition allows the test to succeed + // else if (seq < betterMinSeq) + // BEAST_EXPECTS(!nodeObject, ss.str()); + else + BEAST_EXPECTS(nodeObject, ss.str()); + } } BEAST_EXPECT(maxSeq != lastRotated + deleteInterval); BEAST_EXPECTS(