From 93f6198b5e2cb0b6f183a309002b39163d50e695 Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Tue, 28 Apr 2020 06:47:35 -0700 Subject: [PATCH] Review fixes. --- src/ripple/app/ledger/impl/LedgerMaster.cpp | 4 +- src/ripple/app/misc/SHAMapStore.h | 64 ++++++++++----------- src/ripple/app/misc/SHAMapStoreImp.cpp | 4 +- src/ripple/app/misc/SHAMapStoreImp.h | 1 + 4 files changed, 36 insertions(+), 37 deletions(-) diff --git a/src/ripple/app/ledger/impl/LedgerMaster.cpp b/src/ripple/app/ledger/impl/LedgerMaster.cpp index 73ffb997713..82ee6c1545a 100644 --- a/src/ripple/app/ledger/impl/LedgerMaster.cpp +++ b/src/ripple/app/ledger/impl/LedgerMaster.cpp @@ -50,7 +50,6 @@ #include #include #include -#include #include namespace ripple { @@ -153,7 +152,7 @@ shouldAcquire ( std::uint32_t const candidateLedger, beast::Journal j) { - bool ret = [&]() + bool const ret = [&]() { // Fetch ledger if it may be the current ledger if (candidateLedger >= currentLedger) @@ -164,6 +163,7 @@ shouldAcquire ( return true; // Or if greater than or equal to a specific minimum ledger. + // Do nothing if the minimum ledger to keep online is unknown. return minimumOnline.has_value() && candidateLedger >= *minimumOnline; }(); diff --git a/src/ripple/app/misc/SHAMapStore.h b/src/ripple/app/misc/SHAMapStore.h index d99dd4ae489..fb2de7bb7a7 100644 --- a/src/ripple/app/misc/SHAMapStore.h +++ b/src/ripple/app/misc/SHAMapStore.h @@ -25,7 +25,6 @@ #include #include #include -#include namespace ripple { @@ -69,39 +68,36 @@ class SHAMapStore /** Returns the number of file descriptors that are needed. */ virtual int fdRequired() const = 0; - /** If online deletion is enabled, return the minimum ledger - * to keep online, which is the lower bound for attempting to acquire - * historical ledgers over the peer to peer network. - * If online_delete is not enabled, then the minimum value to - * keep online is the minimum ledger that has been persisted already. - * - * With online_delete,this value is governed by the - * following circumstances: - * - * Upon process startup: - * - * With advisory_delete enabled and online_delete having been executed - * previously: - * Upon process startup, this value is set to one past the value - * that is allowed to be deleted. In other words, anything greater - * than what can be deleted should be acquired from the network and - * then retained. If nothing has yet been deleted, then do not - * rely upon the value of what is allowed to be deleted because it - * could cause fetching of more history than configured by - * ledger_history. - * - * Without advisory_delete or if online_delete has never executed: - * Upon process startup, this value is set to the earliest ledger - * that has been persisted in SQLite. - * - * Each time online_delete executes: - * - * Just prior to clearing SQL databases of historical ledgers, move - * the value forward to one past the greatest ledger being deleted. - * This minimizes fetching of ledgers that are in the process of being - * deleted. - */ - virtual boost::optional minimumOnline() const = 0; + /** The minimum ledger to try and maintain in our database. + + This defines the lower bound for attempting to acquire historical + ledgers over the peer to peer network. + + With online_delete,this value is governed by the following + circumstances: + + - With advisory_delete enabled and online_delete having been executed + previously, this value is set to one past the value that is allowed to + be deleted. In other words, anything greater than what can be deleted + should be acquired from the network and then retained. If nothing has + yet been deleted, then do not rely upon the value of what is allowed + to be deleted because it could cause fetching of more history than + configured by ledger_history. + + - Without advisory_delete or if online_delete has never executed, this + value is set to the earliest ledger that has been persisted in the + SQL database. + + Each time online_delete executes, just prior to clearing SQL databases + of historical ledgers, move the value forward to one past the greatest + ledger being deleted. This minimizes fetching of ledgers that are in + the process of being deleted. + + @return If online deletion is enabled and if it has been executed, + the minimum ledger to keep available; an unseated + optional otherwise. + */ + virtual boost::optional minimumOnline() const = 0; }; //------------------------------------------------------------------------------ diff --git a/src/ripple/app/misc/SHAMapStoreImp.cpp b/src/ripple/app/misc/SHAMapStoreImp.cpp index 06bcf06eafb..463a796de92 100644 --- a/src/ripple/app/misc/SHAMapStoreImp.cpp +++ b/src/ripple/app/misc/SHAMapStoreImp.cpp @@ -752,7 +752,9 @@ SHAMapStoreImp::onChildrenStopped() boost::optional SHAMapStoreImp::minimumOnline() const { - if (deleteInterval_) + // minimumOnline_ with 0 value is equivalent to unknown/not set. + // Don't attempt to acquire ledgers if that value is unknown. + if (deleteInterval_ && minimumOnline_) return minimumOnline_.load(); return app_.getLedgerMaster().minSqlSeq(); } diff --git a/src/ripple/app/misc/SHAMapStoreImp.h b/src/ripple/app/misc/SHAMapStoreImp.h index fa683887954..2214feb097a 100644 --- a/src/ripple/app/misc/SHAMapStoreImp.h +++ b/src/ripple/app/misc/SHAMapStoreImp.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include