From 45ff08b6aa13128be57ae2b0a2cdef0c434fd8d7 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Thu, 5 May 2016 01:49:55 -0700 Subject: [PATCH] Fix advisory delete affect on history acquisition (RIPD-1112): * Revert 0efb929898270fbebf036cab36902cddb32889ee * Advisory delete setting of 0 (never) does not affect history fetching The previous commit addressing RIPD-1112 could interact with advisory delete and cause some history not to be acquired even configured to acquire. This reverts that commit and provides a better fix. The advisory delete setting protects ledgers from being removed by online delete by exempting them until they are approved for purge by administrative command. However, not connecting this with history acquisition could cause new ledgers in the protected range not to be acquired if the server loses sync. With this change, the default advisory delete setting, zero (never) causes the regular server history setting to control the acquisition of history. Setting advisory delete to a value greater than zero, if advisory delete is enabled, will cause the server to fetch and maintain history back to that point. This should produce sane behavior across server restarts, losses of sync, and so on. You can no longer use the "hack" of setting advisory delete to zero to tell the server to fetch and keep as much history as possible, but you can achieve the same effect by setting it to one. --- src/ripple/app/ledger/impl/LedgerMaster.cpp | 15 +++++++++------ src/ripple/app/misc/SHAMapStoreImp.h | 4 ++++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/ripple/app/ledger/impl/LedgerMaster.cpp b/src/ripple/app/ledger/impl/LedgerMaster.cpp index db1b57121..500eefa6b 100644 --- a/src/ripple/app/ledger/impl/LedgerMaster.cpp +++ b/src/ripple/app/ledger/impl/LedgerMaster.cpp @@ -1560,12 +1560,15 @@ LedgerMaster::shouldAcquire ( std::uint32_t const ledgerHistoryIndex, std::uint32_t const candidateLedger) const { - // If the ledger could be the current ledger acquire it. - // Otherwise, acquire it only if it's within our range of - // desired history and we wouldn't delete it if we had it. - bool ret = (candidateLedger >= currentLedger) || - ((candidateLedger >= ledgerHistoryIndex) && - ((currentLedger - candidateLedger) <= ledgerHistory)); + + // Fetch ledger if it might be the current ledger, + // is requested by the advisory delete setting, or + // is within our configured history range + + bool ret (candidateLedger >= currentLedger || + ((ledgerHistoryIndex > 0) && + (candidateLedger > ledgerHistoryIndex)) || + (currentLedger - candidateLedger) <= ledgerHistory); JLOG (m_journal.trace()) << "Missing ledger " diff --git a/src/ripple/app/misc/SHAMapStoreImp.h b/src/ripple/app/misc/SHAMapStoreImp.h index 89e34dec3..f4cb75a02 100644 --- a/src/ripple/app/misc/SHAMapStoreImp.h +++ b/src/ripple/app/misc/SHAMapStoreImp.h @@ -150,12 +150,16 @@ public: return setup_.advisoryDelete; } + // All ledgers prior to this one are eligible + // for deletion in the next rotation LedgerIndex getLastRotated() override { return state_db_.getState().lastRotated; } + // All ledgers before and including this are unprotected + // and online delete may delete them if appropriate LedgerIndex getCanDelete() override {