diff --git a/cfg/xrpld-example.cfg b/cfg/xrpld-example.cfg index 9e334e6f4f3..370195f3d0b 100644 --- a/cfg/xrpld-example.cfg +++ b/cfg/xrpld-example.cfg @@ -1059,10 +1059,11 @@ # The online delete process checks periodically # that xrpld is still in sync with the network, # and that the validated ledger is less than -# 'age_threshold_seconds' old. If not, then continue +# 'age_threshold_seconds' old, and that all +# recent ledgers are available. If not, then continue # sleeping for this number of seconds and # checking until healthy. -# Default is 5. +# Default is 2. # # Notes: # The 'node_db' entry configures the primary, persistent storage. diff --git a/src/test/app/LedgerMaster_test.cpp b/src/test/app/LedgerMaster_test.cpp index 3cf9b3a9d9e..ece25356fd0 100644 --- a/src/test/app/LedgerMaster_test.cpp +++ b/src/test/app/LedgerMaster_test.cpp @@ -5,17 +5,21 @@ #include #include +#include #include +#include #include #include #include +#include #include #include #include #include #include +#include #include namespace xrpl::test { @@ -111,6 +115,71 @@ class LedgerMaster_test : public beast::unit_test::Suite } } + void + testCompleteLedgerRange(FeatureBitset features) + { + // Note that this test is intentionally very similar to + // SHAMapStore_test::testLedgerGaps, but has a different + // focus. + + testcase("Complete Ledger operations"); + + using namespace test::jtx; + + auto const deleteInterval = 8; + + Env env{*this, envconfig(onlineDelete, deleteInterval)}; + + auto const alice = Account("alice"); + env.fund(XRP(1000), alice); + env.close(); + + auto& lm = env.app().getLedgerMaster(); + LedgerIndex minSeq = 2; + LedgerIndex maxSeq = env.closed()->header().seq; + auto& store = env.app().getSHAMapStore(); + BEAST_EXPECT(store.rendezvous()); + LedgerIndex lastRotated = store.getLastRotated(); + BEAST_EXPECTS(maxSeq == 3, to_string(maxSeq)); + BEAST_EXPECTS(lm.getCompleteLedgers() == "2-3", lm.getCompleteLedgers()); + BEAST_EXPECTS(lastRotated == 3, to_string(lastRotated)); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq, maxSeq) == 0); + BEAST_EXPECT(minSeq + 1 > maxSeq - 1); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq - 1, maxSeq + 1) == 2); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq - 2, maxSeq - 2) == 2); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq + 2, maxSeq + 2) == 2); + + // Close enough ledgers to rotate a few times + for (int i = 0; i < 24; ++i) + { + for (int t = 0; t < 3; ++t) + { + env(noop(alice)); + } + env.close(); + BEAST_EXPECT(store.rendezvous()); + + ++maxSeq; + + if (maxSeq == lastRotated + deleteInterval) + { + minSeq = lastRotated; + lastRotated = maxSeq; + } + BEAST_EXPECTS( + env.closed()->header().seq == maxSeq, to_string(env.closed()->header().seq)); + BEAST_EXPECTS(store.getLastRotated() == lastRotated, to_string(store.getLastRotated())); + std::stringstream expectedRange; + expectedRange << minSeq << "-" << maxSeq; + BEAST_EXPECTS(lm.getCompleteLedgers() == expectedRange.str(), lm.getCompleteLedgers()); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq, maxSeq) == 0); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq + 1, maxSeq - 1) == 0); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq - 1, maxSeq + 1) == 2); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq - 2, maxSeq - 2) == 2); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq + 2, maxSeq + 2) == 2); + } + } + public: void run() override @@ -124,6 +193,7 @@ class LedgerMaster_test : public beast::unit_test::Suite testWithFeats(FeatureBitset features) { testTxnIdFromIndex(features); + testCompleteLedgerRange(features); } }; diff --git a/src/test/app/SHAMapStore_test.cpp b/src/test/app/SHAMapStore_test.cpp index c219bd8737d..267fbbc9eb8 100644 --- a/src/test/app/SHAMapStore_test.cpp +++ b/src/test/app/SHAMapStore_test.cpp @@ -1,7 +1,9 @@ #include #include #include +#include +#include #include #include #include @@ -26,12 +28,15 @@ #include #include +#include #include #include #include #include #include +#include #include +#include #include namespace xrpl::test { @@ -43,10 +48,7 @@ class SHAMapStore_test : public beast::unit_test::Suite static auto onlineDelete(std::unique_ptr cfg) { - cfg->ledgerHistory = kDeleteInterval; - auto& section = cfg->section(Sections::kNodeDatabase); - section.set(Keys::kOnlineDelete, std::to_string(kDeleteInterval)); - return cfg; + return jtx::onlineDelete(std::move(cfg), kDeleteInterval); } static auto @@ -144,11 +146,11 @@ class SHAMapStore_test : public beast::unit_test::Suite auto& store = env.app().getSHAMapStore(); int ledgerSeq = 3; - store.rendezvous(); + BEAST_EXPECT(store.rendezvous()); BEAST_EXPECT(!store.getLastRotated()); env.close(); - store.rendezvous(); + BEAST_EXPECT(store.rendezvous()); auto ledger = env.rpc("ledger", "validated"); BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++))); @@ -228,7 +230,7 @@ class SHAMapStore_test : public beast::unit_test::Suite BEAST_EXPECT(goodLedger(env, ledger, std::to_string(kDeleteInterval + 4))); } - store.rendezvous(); + BEAST_EXPECT(store.rendezvous()); BEAST_EXPECT(store.getLastRotated() == kDeleteInterval + 3); lastRotated = store.getLastRotated(); @@ -255,7 +257,7 @@ class SHAMapStore_test : public beast::unit_test::Suite !getHash(ledgers[i]).empty()); } - store.rendezvous(); + BEAST_EXPECT(store.rendezvous()); BEAST_EXPECT(store.getLastRotated() == kDeleteInterval + lastRotated); @@ -293,7 +295,7 @@ class SHAMapStore_test : public beast::unit_test::Suite BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true)); } - store.rendezvous(); + BEAST_EXPECT(store.rendezvous()); // The database will always have back to ledger 2, // regardless of lastRotated. @@ -308,7 +310,7 @@ class SHAMapStore_test : public beast::unit_test::Suite BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++), true)); } - store.rendezvous(); + BEAST_EXPECT(store.rendezvous()); ledgerCheck(env, ledgerSeq - lastRotated, lastRotated); BEAST_EXPECT(lastRotated != store.getLastRotated()); @@ -324,7 +326,7 @@ class SHAMapStore_test : public beast::unit_test::Suite BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true)); } - store.rendezvous(); + BEAST_EXPECT(store.rendezvous()); ledgerCheck(env, kDeleteInterval + 1, lastRotated); BEAST_EXPECT(lastRotated != store.getLastRotated()); @@ -363,7 +365,7 @@ class SHAMapStore_test : public beast::unit_test::Suite BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true)); } - store.rendezvous(); + BEAST_EXPECT(store.rendezvous()); ledgerCheck(env, ledgerSeq - 2, 2); BEAST_EXPECT(lastRotated == store.getLastRotated()); @@ -373,7 +375,7 @@ class SHAMapStore_test : public beast::unit_test::Suite BEAST_EXPECT(!RPC::containsError(canDelete[jss::result])); BEAST_EXPECT(canDelete[jss::result][jss::can_delete] == ledgerSeq + (kDeleteInterval / 2)); - store.rendezvous(); + BEAST_EXPECT(store.rendezvous()); ledgerCheck(env, ledgerSeq - 2, 2); BEAST_EXPECT(store.getLastRotated() == lastRotated); @@ -386,7 +388,7 @@ class SHAMapStore_test : public beast::unit_test::Suite BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++), true)); } - store.rendezvous(); + BEAST_EXPECT(store.rendezvous()); ledgerCheck(env, ledgerSeq - lastRotated, lastRotated); @@ -402,7 +404,7 @@ class SHAMapStore_test : public beast::unit_test::Suite BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true)); } - store.rendezvous(); + BEAST_EXPECT(store.rendezvous()); BEAST_EXPECT(store.getLastRotated() == lastRotated); @@ -414,7 +416,7 @@ class SHAMapStore_test : public beast::unit_test::Suite BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++), true)); } - store.rendezvous(); + BEAST_EXPECT(store.rendezvous()); ledgerCheck(env, ledgerSeq - firstBatch, firstBatch); @@ -436,7 +438,7 @@ class SHAMapStore_test : public beast::unit_test::Suite BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true)); } - store.rendezvous(); + BEAST_EXPECT(store.rendezvous()); BEAST_EXPECT(store.getLastRotated() == lastRotated); @@ -448,7 +450,7 @@ class SHAMapStore_test : public beast::unit_test::Suite BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++), true)); } - store.rendezvous(); + BEAST_EXPECT(store.rendezvous()); ledgerCheck(env, ledgerSeq - lastRotated, lastRotated); @@ -469,7 +471,7 @@ class SHAMapStore_test : public beast::unit_test::Suite BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true)); } - store.rendezvous(); + BEAST_EXPECT(store.rendezvous()); BEAST_EXPECT(store.getLastRotated() == lastRotated); @@ -481,7 +483,7 @@ class SHAMapStore_test : public beast::unit_test::Suite BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++), true)); } - store.rendezvous(); + BEAST_EXPECT(store.rendezvous()); ledgerCheck(env, ledgerSeq - lastRotated, lastRotated); @@ -604,6 +606,176 @@ class SHAMapStore_test : public beast::unit_test::Suite BEAST_EXPECT(dbr->getName() == "3"); } + void + testLedgerGaps() + { + // Note that this test is intentionally very similar to + // LedgerMaster_test::testCompleteLedgerRange, but has a different + // focus. + + testcase("Wait for ledger gaps to fill in"); + + using namespace test::jtx; + + Env env{*this, envconfig(onlineDelete)}; + + auto failureMessage = [&](char const* label, auto expected, auto actual) { + std::stringstream ss; + ss << label << ": Expected: " << expected << ", Got: " << actual; + return ss.str(); + }; + + auto const alice = Account("alice"); + env.fund(XRP(1000), alice); + env.close(); + + auto& lm = env.app().getLedgerMaster(); + LedgerIndex minSeq = 2; + LedgerIndex maxSeq = env.closed()->header().seq; + auto& store = env.app().getSHAMapStore(); + LedgerIndex lastRotated = store.getLastRotated(); + while (lastRotated != 3) + { + BEAST_EXPECT(store.rendezvous()); + lastRotated = store.getLastRotated(); + } + BEAST_EXPECTS(maxSeq == 3, std::to_string(maxSeq)); + BEAST_EXPECTS(lm.getCompleteLedgers() == "2-3", lm.getCompleteLedgers()); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq, maxSeq) == 0); + BEAST_EXPECT(minSeq + 1 > maxSeq - 1); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq - 1, maxSeq + 1) == 2); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq - 2, maxSeq - 2) == 2); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq + 2, maxSeq + 2) == 2); + + // Close enough ledgers to rotate a few times + while (maxSeq < 20) + { + for (int t = 0; t < 3; ++t) + { + env(noop(alice)); + } + env.close(); + BEAST_EXPECT(store.rendezvous()); + + ++maxSeq; + + if (maxSeq + 1 == lastRotated + kDeleteInterval) + { + using namespace std::chrono_literals; + + // The next ledger will trigger a rotation. Delete the + // current ledger from LedgerMaster. + std::this_thread::sleep_for(100ms); + LedgerIndex const deleteSeq = maxSeq; + { + std::size_t iterations = 30; + while (!lm.haveLedger(deleteSeq) && --iterations > 0) + { + std::this_thread::sleep_for(100ms); + } + // Even the slowest machines should be able to finalize deleteSeq within 10 + // loops (400ms). If this test ever actually fails feel free to lower this + // cutoff. The intent of this test is to flag if the loop takes a very long + // time, but still allow the rest of this function to finish. + BEAST_EXPECTS(iterations > 20, std::to_string(iterations)); + if (!BEAST_EXPECT(lm.haveLedger(deleteSeq))) + return; + } + lm.clearLedger(deleteSeq); + + auto expectedRange = [](auto minSeq, auto deleteSeq, auto maxSeq) { + std::stringstream expectedRange; + expectedRange << minSeq << "-" << (deleteSeq - 1); + if (deleteSeq + 1 == maxSeq) + { + expectedRange << "," << maxSeq; + } + else if (deleteSeq < maxSeq) + { + expectedRange << "," << (deleteSeq + 1) << "-" << maxSeq; + } + return expectedRange.str(); + }; + BEAST_EXPECTS( + lm.getCompleteLedgers() == expectedRange(minSeq, deleteSeq, maxSeq), + failureMessage( + "Complete ledgers", + expectedRange(minSeq, deleteSeq, maxSeq), + lm.getCompleteLedgers())); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq, maxSeq) == 1); + + // Close another ledger, which will trigger a rotation, but the + // rotation will be stuck until the missing ledger is filled in. + env.close(); + // Do not call rendezvous() here without a timeout; it will block until the missing + // ledger is backfilled. That will not happen automatically. It's a manual step that + // is done later in this test. + ++maxSeq; + + // Nothing has changed + BEAST_EXPECTS( + store.getLastRotated() == lastRotated, + failureMessage("lastRotated", lastRotated, store.getLastRotated())); + BEAST_EXPECTS( + lm.getCompleteLedgers() == expectedRange(minSeq, deleteSeq, maxSeq), + failureMessage( + "Complete ledgers", + expectedRange(minSeq, deleteSeq, maxSeq), + lm.getCompleteLedgers())); + + // Close 5 more ledgers, waiting a little bit in between to + // simulate the ledger making progress while online delete waits + // for the missing ledger to be filled in. + // This ensures the healthWait check has time to run and + // detect the gap. + for (int l = 0; l < 5; ++l) + { + env.close(); + ++maxSeq; + // Nothing has changed + BEAST_EXPECTS( + store.getLastRotated() == lastRotated, + failureMessage("lastRotated", lastRotated, store.getLastRotated())); + BEAST_EXPECTS( + lm.getCompleteLedgers() == expectedRange(minSeq, deleteSeq, maxSeq), + failureMessage( + "Complete Ledgers", + expectedRange(minSeq, deleteSeq, maxSeq), + lm.getCompleteLedgers())); + // The Store is "stuck" in healthWait() and won't finish the run() loop until + // it's backfilled + BEAST_EXPECT(!store.rendezvous(100ms)); + } + + // Put the missing ledger back in LedgerMaster + lm.setLedgerRangePresent(deleteSeq, deleteSeq); + + // Wait for the rotation to finish + BEAST_EXPECT(store.rendezvous()); + + minSeq = lastRotated; + lastRotated = deleteSeq + 1; + } + BEAST_EXPECT(maxSeq != lastRotated + kDeleteInterval); + BEAST_EXPECTS( + env.closed()->header().seq == maxSeq, + failureMessage("maxSeq", maxSeq, env.closed()->header().seq)); + BEAST_EXPECTS( + store.getLastRotated() == lastRotated, + failureMessage("lastRotated", lastRotated, store.getLastRotated())); + std::stringstream expectedRange; + expectedRange << minSeq << "-" << maxSeq; + BEAST_EXPECTS( + lm.getCompleteLedgers() == expectedRange.str(), + failureMessage("CompleteLedgers", expectedRange.str(), lm.getCompleteLedgers())); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq, maxSeq) == 0); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq + 1, maxSeq - 1) == 0); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq - 1, maxSeq + 1) == 2); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq - 2, maxSeq - 2) == 2); + BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq + 2, maxSeq + 2) == 2); + } + } + void run() override { @@ -611,6 +783,7 @@ class SHAMapStore_test : public beast::unit_test::Suite testAutomatic(); testCanDelete(); testRotate(); + testLedgerGaps(); } }; diff --git a/src/test/jtx/envconfig.h b/src/test/jtx/envconfig.h index 1f920fca589..5ad24e25c43 100644 --- a/src/test/jtx/envconfig.h +++ b/src/test/jtx/envconfig.h @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -62,6 +63,19 @@ envconfig(F&& modfunc, Args&&... args) return modfunc(envconfig(), std::forward(args)...); } +/** + * @brief adjust config to enable online_delete + * + * @param cfg config instance to be modified + * + * @param deleteInterval how many new ledgers should be available before + * rotating. Defaults to 8, because the standalone minimum is 8. + * + * @return unique_ptr to Config instance + */ +std::unique_ptr +onlineDelete(std::unique_ptr cfg, std::uint32_t deleteInterval = 8); + /** * @brief adjust config so no admin ports are enabled * diff --git a/src/test/jtx/impl/envconfig.cpp b/src/test/jtx/impl/envconfig.cpp index bc65738b441..14690058ec7 100644 --- a/src/test/jtx/impl/envconfig.cpp +++ b/src/test/jtx/impl/envconfig.cpp @@ -7,8 +7,10 @@ #include #include +#include #include #include +#include #include namespace xrpl::test { @@ -60,6 +62,15 @@ setupConfigForUnitTests(Config& cfg) namespace jtx { +std::unique_ptr +onlineDelete(std::unique_ptr cfg, std::uint32_t deleteInterval) +{ + cfg->ledgerHistory = deleteInterval; + auto& section = cfg->section(Sections::kNodeDatabase); + section.set(Keys::kOnlineDelete, std::to_string(deleteInterval)); + return cfg; +} + std::unique_ptr noAdmin(std::unique_ptr cfg) { diff --git a/src/xrpld/app/ledger/LedgerMaster.h b/src/xrpld/app/ledger/LedgerMaster.h index 32163fd57b9..462073a4f35 100644 --- a/src/xrpld/app/ledger/LedgerMaster.h +++ b/src/xrpld/app/ledger/LedgerMaster.h @@ -123,7 +123,10 @@ class LedgerMaster : public AbstractFetchPackContainer failedSave(std::uint32_t seq, uint256 const& hash); std::string - getCompleteLedgers(); + getCompleteLedgers() const; + + std::size_t + missingFromCompleteLedgerRange(LedgerIndex first, LedgerIndex last) const; /** * Apply held transactions to the open ledger @@ -348,7 +351,7 @@ class LedgerMaster : public AbstractFetchPackContainer // A set of transactions to replay during the next close std::unique_ptr replayData_; - std::recursive_mutex completeLock_; + std::recursive_mutex mutable completeLock_; RangeSet completeLedgers_; // Publish thread is running. diff --git a/src/xrpld/app/ledger/detail/LedgerMaster.cpp b/src/xrpld/app/ledger/detail/LedgerMaster.cpp index 2bd83b0f18b..589d592a294 100644 --- a/src/xrpld/app/ledger/detail/LedgerMaster.cpp +++ b/src/xrpld/app/ledger/detail/LedgerMaster.cpp @@ -57,6 +57,7 @@ #include #include +#include #include #include @@ -1576,12 +1577,33 @@ LedgerMaster::getPublishedLedger() } std::string -LedgerMaster::getCompleteLedgers() +LedgerMaster::getCompleteLedgers() const { std::scoped_lock const sl(completeLock_); return to_string(completeLedgers_); } +std::size_t +LedgerMaster::missingFromCompleteLedgerRange(LedgerIndex first, LedgerIndex last) const +{ + if (first > last) + { + // LCOV_EXCL_START + UNREACHABLE("xrpl::LedgerMaster::missingFromCompleteLedgerRange : invalid parameters"); + return 0; + // LCOV_EXCL_STOP + } + + RangeSet const target{range(first, last)}; + + auto const missing = [&target, this] { + std::scoped_lock const sl(completeLock_); + return target - completeLedgers_; + }(); + + return boost::icl::size(missing); +} + std::optional LedgerMaster::getCloseTimeBySeq(LedgerIndex ledgerIndex) { diff --git a/src/xrpld/app/misc/SHAMapStore.h b/src/xrpld/app/misc/SHAMapStore.h index df696c685f8..603fd4f0c6e 100644 --- a/src/xrpld/app/misc/SHAMapStore.h +++ b/src/xrpld/app/misc/SHAMapStore.h @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -34,8 +35,8 @@ class SHAMapStore virtual void start() = 0; - virtual void - rendezvous() const = 0; + [[nodiscard]] virtual bool + rendezvous(std::optional const& timeout = {}) const = 0; virtual void stop() = 0; diff --git a/src/xrpld/app/misc/SHAMapStoreImp.cpp b/src/xrpld/app/misc/SHAMapStoreImp.cpp index 9b5f412fc5f..bfe86a9205b 100644 --- a/src/xrpld/app/misc/SHAMapStoreImp.cpp +++ b/src/xrpld/app/misc/SHAMapStoreImp.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -32,6 +33,8 @@ #include #include +#include +#include #include #include #include @@ -236,14 +239,21 @@ SHAMapStoreImp::onLedgerClosed(std::shared_ptr const& ledger) cond_.notify_one(); } -void -SHAMapStoreImp::rendezvous() const +bool +SHAMapStoreImp::rendezvous(std::optional const& timeout) const { if (!working_) - return; + return true; + + auto notWorking = [&] { return !working_; }; std::unique_lock lock(mutex_); - rendezvous_.wait(lock, [&] { return !working_; }); + if (timeout) + { + return rendezvous_.wait_for(lock, *timeout, notWorking); + } + rendezvous_.wait(lock, notWorking); + return true; } int @@ -330,6 +340,16 @@ SHAMapStoreImp::run() bool const readyToRotate = validatedSeq >= lastRotated + deleteInterval_ && canDelete_ >= lastRotated - 1 && healthWait() == HealthResult::KeepGoing; + { + JLOG(journal_.trace()) << "run: Setting lastGoodValidatedLedger_ to " << validatedSeq; + // Note that this is set after the healthWait() check, so that we + // don't start the rotation until the validated ledger is fully + // processed. It is not guaranteed to be done at this point. It also + // allows the testLedgerGaps unit test to work. + std::unique_lock const lock(mutex_); + lastGoodValidatedLedger_ = validatedSeq; + } + // will delete up to (not including) lastRotated if (readyToRotate) { @@ -337,7 +357,8 @@ SHAMapStoreImp::run() << lastRotated << " deleteInterval " << deleteInterval_ << " canDelete_ " << canDelete_ << " state " << app_.getOPs().strOperatingMode(false) << " age " - << ledgerMaster_->getValidatedLedgerAge().count() << 's'; + << ledgerMaster_->getValidatedLedgerAge().count() + << "s. Complete ledgers: " << ledgerMaster_->getCompleteLedgers(); clearPrior(lastRotated); if (healthWait() == HealthResult::Stopping) @@ -412,7 +433,9 @@ SHAMapStoreImp::run() clearCaches(validatedSeq); }); - JLOG(journal_.warn()) << "finished rotation " << validatedSeq; + JLOG(journal_.warn()) << "finished rotation. validatedSeq: " << validatedSeq + << ", lastRotated: " << lastRotated + << ". Complete ledgers: " << ledgerMaster_->getCompleteLedgers(); } } } @@ -656,20 +679,70 @@ SHAMapStoreImp::clearPrior(LedgerIndex lastRotated) SHAMapStoreImp::HealthResult SHAMapStoreImp::healthWait() { - auto age = ledgerMaster_->getValidatedLedgerAge(); - OperatingMode mode = netOPs_->getOperatingMode(); - std::unique_lock lock(mutex_); - while (!stop_ && (mode != OperatingMode::FULL || age > ageThreshold_)) - { - lock.unlock(); - JLOG(journal_.warn()) << "Waiting " << recoveryWaitTime_.count() - << "s for node to stabilize. state: " - << app_.getOPs().strOperatingMode(mode, false) << ". age " - << age.count() << 's'; - std::this_thread::sleep_for(recoveryWaitTime_); + // Gets the current status of the server from ledgerMaster_ and netOPs_. Must be called + // while mutex_ is unlocked to avoid unlikely, but possible, deadlock with ledgerMaster_'s + // completeLock_. + // Releasing the lock may mean that status will be slightly out of date when the lock is + // reacquired, but it's close enough. In a normal rotation, healthWait() is called frequently, + // so a false positive will be detected on the next call, and a false negative will be detected + // in the next loop iteration. Database rotation is important, but not timely, so an extra + // delay is fine. + auto readServerStatus = [this]( + LedgerIndex& index, + std::chrono::seconds& age, + OperatingMode& mode, + std::size_t& numMissing, + LedgerIndex const lowerBound, + ScopeUnlock const&) { + index = ledgerMaster_->getValidLedgerIndex(); age = ledgerMaster_->getValidatedLedgerAge(); mode = netOPs_->getOperatingMode(); - lock.lock(); + + numMissing = + lowerBound == 0 ? 0 : ledgerMaster_->missingFromCompleteLedgerRange(lowerBound, index); + }; + + // Tracked server status properties + LedgerIndex index = 0; + std::chrono::seconds age; + OperatingMode mode = OperatingMode::DISCONNECTED; + std::size_t numMissing = 0; + LedgerIndex lastLedger = 0; + + std::unique_lock lock(mutex_); + + auto const waitTime = recoveryWaitTime_; + auto const ageThreshold = ageThreshold_; + { + auto const lowerBound = lastGoodValidatedLedger_; + + ScopeUnlock const unlock(lock); + + readServerStatus(index, age, mode, numMissing, lowerBound, unlock); + } + while (!stop_ && (mode != OperatingMode::FULL || age > ageThreshold || numMissing > 0)) + { + // this value shouldn't change, so grab it while we have the + // lock + auto const lowerBound = lastGoodValidatedLedger_; + + ScopeUnlock const unlock(lock); + + auto const stream = std::invoke([mode, age, ageThreshold, index, lastLedger, this]() { + if (mode != OperatingMode::FULL || age > ageThreshold) + return journal_.warn(); + if (index != lastLedger) + return journal_.trace(); + return journal_.info(); + }); + JLOG(stream) << "Waiting " << waitTime.count() << "s for node to stabilize. state: " + << app_.getOPs().strOperatingMode(mode, false) << ". age " << age.count() + << "s. Missing ledgers: " << numMissing << ". Expect: " << lowerBound << "-" + << index << ". Complete ledgers: " << ledgerMaster_->getCompleteLedgers(); + std::this_thread::sleep_for(waitTime); + + readServerStatus(index, age, mode, numMissing, lowerBound, unlock); + lastLedger = index; } return stop_ ? HealthResult::Stopping : HealthResult::KeepGoing; diff --git a/src/xrpld/app/misc/SHAMapStoreImp.h b/src/xrpld/app/misc/SHAMapStoreImp.h index a0ca59ecc89..e02f64f56fa 100644 --- a/src/xrpld/app/misc/SHAMapStoreImp.h +++ b/src/xrpld/app/misc/SHAMapStoreImp.h @@ -88,6 +88,11 @@ class SHAMapStoreImp : public SHAMapStore std::thread thread_; bool stop_ = false; bool healthy_ = true; + // Used to prevent ledger gaps from forming during online deletion. Keeps + // track of the last validated ledger that was processed without gaps. There + // are no guarantees about gaps while online delete is not running. For + // that, use advisory_delete and check for gaps externally. + LedgerIndex lastGoodValidatedLedger_ = 0; mutable std::condition_variable cond_; mutable std::condition_variable rendezvous_; mutable std::mutex mutex_; @@ -102,12 +107,12 @@ class SHAMapStoreImp : public SHAMapStore std::chrono::milliseconds backOff_{100}; std::chrono::seconds ageThreshold_{60}; /** - * If the node is out of sync during an online_delete healthWait() - * call, sleep the thread for this time, and continue checking until - * recovery. + * If the node is out of sync, or any recent ledgers are not + * available during an online_delete healthWait() call, sleep + * the thread for this time, and continue checking until recovery. * See also: "recovery_wait_seconds" in xrpld-example.cfg */ - std::chrono::seconds recoveryWaitTime_{5}; + std::chrono::seconds recoveryWaitTime_{2}; // these do not exist upon SHAMapStore creation, but do exist // as of run() or before @@ -163,8 +168,8 @@ class SHAMapStoreImp : public SHAMapStore void onLedgerClosed(std::shared_ptr const& ledger) override; - void - rendezvous() const override; + bool + rendezvous(std::optional const& timeout = {}) const override; int fdRequired() const override;