Skip to content

Commit

Permalink
Fix historical ledger acquisition when using online deletion:
Browse files Browse the repository at this point in the history 
Commit e257a22 introduced changes in the logic used to acquire historical
ledgers. The logic could cause historical ledgers to be acquired only since
the last online deletion interval instead of the configured value to allow
deletion.
  • Loading branch information
@mtrippled @manojsdoshi
mtrippled authored and manojsdoshi committed May 27, 2020
1 parent 99f5193 commit 17412d1
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 46 deletions.
4 changes: 4 additions & 0 deletions src/ripple/app/ledger/LedgerMaster.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,10 @@ class LedgerMaster : public Stoppable, public AbstractFetchPackContainer
return !mValidLedger.empty();
}

// Returns the minimum ledger sequence in SQL database, if any.
boost::optional<LedgerIndex>
minSqlSeq();

private:
void
setValidLedger(std::shared_ptr<Ledger const> const& l);
Expand Down
41 changes: 16 additions & 25 deletions src/ripple/app/ledger/impl/LedgerMaster.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,29 +143,17 @@ static constexpr std::chrono::minutes MAX_LEDGER_AGE_ACQUIRE{1};
// Don't acquire history if write load is too high
static constexpr int MAX_WRITE_LOAD_ACQUIRE{8192};

// Helper function for LedgerMaster::doAdvance()
// Returns the minimum ledger sequence in SQL database, if any.
static boost::optional<LedgerIndex>
minSqlSeq(Application& app)
{
boost::optional<LedgerIndex> seq;
auto db = app.getLedgerDB().checkoutDb();
*db << "SELECT MIN(LedgerSeq) FROM Ledgers", soci::into(seq);
return seq;
}

// Helper function for LedgerMaster::doAdvance()
// Return true if candidateLedger should be fetched from the network.
static bool
shouldAcquire(
std::uint32_t const currentLedger,
std::uint32_t const ledgerHistory,
boost::optional<LedgerIndex> minSeq,
std::uint32_t const lastRotated,
boost::optional<LedgerIndex> const minimumOnline,
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)
return true;
Expand All @@ -174,15 +162,9 @@ shouldAcquire(
if (currentLedger - candidateLedger <= ledgerHistory)
return true;

// Or it's greater than or equal to both:
// - the minimum persisted ledger or the maximum possible
// sequence value, if no persisted ledger, and
// - minimum ledger that will be persisted as of the next online
// deletion interval, or 1 if online deletion is disabled.
return candidateLedger >=
std::max(
minSeq.value_or(std::numeric_limits<LedgerIndex>::max()),
lastRotated + 1);
// 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;
}();

JLOG(j.trace()) << "Missing ledger " << candidateLedger
Expand Down Expand Up @@ -1899,8 +1881,7 @@ LedgerMaster::doAdvance(std::unique_lock<std::recursive_mutex>& sl)
shouldAcquire(
mValidLedgerSeq,
ledger_history_,
minSqlSeq(app_),
app_.getSHAMapStore().getLastRotated(),
app_.getSHAMapStore().minimumOnline(),
*missing,
m_journal))
{
Expand Down Expand Up @@ -2149,4 +2130,14 @@ LedgerMaster::getFetchPackCacheSize() const
return fetch_packs_.getCacheSize();
}

// Returns the minimum ledger sequence in SQL database, if any.
boost::optional<LedgerIndex>
LedgerMaster::minSqlSeq()
{
boost::optional<LedgerIndex> seq;
auto db = app_.getLedgerDB().checkoutDb();
*db << "SELECT MIN(LedgerSeq) FROM Ledgers", soci::into(seq);
return seq;
}

} // namespace ripple
20 changes: 20 additions & 0 deletions src/ripple/app/misc/SHAMapStore.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <ripple/core/Stoppable.h>
#include <ripple/nodestore/Manager.h>
#include <ripple/protocol/ErrorCodes.h>
#include <boost/optional.hpp>

namespace ripple {

Expand Down Expand Up @@ -74,6 +75,25 @@ class SHAMapStore : public Stoppable
/** Returns the number of file descriptors that are needed. */
virtual int
fdRequired() 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.
If online_delete is enabled, then each time online_delete executes
and 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. Without online_delete or before online_delete is
executed, this value is always the minimum value persisted in the
ledger database, if any.
@return The minimum ledger sequence to keep online based on the
description above. If not set, then an unseated optional.
*/
virtual boost::optional<LedgerIndex>
minimumOnline() const = 0;
};

//------------------------------------------------------------------------------
Expand Down
37 changes: 25 additions & 12 deletions src/ripple/app/misc/SHAMapStoreImp.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ void
SHAMapStoreImp::run()
{
beast::setCurrentThreadName("SHAMapStore");
lastRotated_ = state_db_.getState().lastRotated;
LedgerIndex lastRotated = state_db_.getState().lastRotated;
netOPs_ = &app_.getOPs();
ledgerMaster_ = &app_.getLedgerMaster();
fullBelowCache_ = &app_.family().fullbelow();
Expand Down Expand Up @@ -340,19 +340,19 @@ SHAMapStoreImp::run()
}

LedgerIndex const validatedSeq = validatedLedger->info().seq;
if (!lastRotated_)
if (!lastRotated)
{
lastRotated_ = validatedSeq;
state_db_.setLastRotated(lastRotated_);
lastRotated = validatedSeq;
state_db_.setLastRotated(lastRotated);
}

// will delete up to (not including) lastRotated_
if (validatedSeq >= lastRotated_ + deleteInterval_ &&
canDelete_ >= lastRotated_ - 1)
// will delete up to (not including) lastRotated
if (validatedSeq >= lastRotated + deleteInterval_ &&
canDelete_ >= lastRotated - 1)
{
JLOG(journal_.warn())
<< "rotating validatedSeq " << validatedSeq << " lastRotated_ "
<< lastRotated_ << " deleteInterval " << deleteInterval_
<< "rotating validatedSeq " << validatedSeq << " lastRotated "
<< lastRotated << " deleteInterval " << deleteInterval_
<< " canDelete_ " << canDelete_;

switch (health())
Expand All @@ -366,7 +366,7 @@ SHAMapStoreImp::run()
default:;
}

clearPrior(lastRotated_);
clearPrior(lastRotated);
switch (health())
{
case Health::stopping:
Expand Down Expand Up @@ -426,14 +426,14 @@ SHAMapStoreImp::run()
default:;
}

lastRotated_ = validatedSeq;
lastRotated = validatedSeq;

dbRotating_->rotateWithLock(
[&](std::string const& writableBackendName) {
SavedState savedState;
savedState.writableDb = newBackend->getName();
savedState.archiveDb = writableBackendName;
savedState.lastRotated = lastRotated_;
savedState.lastRotated = lastRotated;
state_db_.setState(savedState);

clearCaches(validatedSeq);
Expand Down Expand Up @@ -624,6 +624,9 @@ SHAMapStoreImp::clearPrior(LedgerIndex lastRotated)
if (health())
return;

// Do not allow ledgers to be acquired from the network
// that are about to be deleted.
minimumOnline_ = lastRotated + 1;
ledgerMaster_->clearPriorLedgers(lastRotated);
if (health())
return;
Expand Down Expand Up @@ -715,6 +718,16 @@ SHAMapStoreImp::onChildrenStopped()
}
}

boost::optional<LedgerIndex>
SHAMapStoreImp::minimumOnline() const
{
// 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();
}

//------------------------------------------------------------------------------

std::unique_ptr<SHAMapStore>
Expand Down
15 changes: 6 additions & 9 deletions src/ripple/app/misc/SHAMapStoreImp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
#include <ripple/app/misc/SHAMapStore.h>
#include <ripple/core/DatabaseCon.h>
#include <ripple/nodestore/DatabaseRotating.h>

#include <atomic>
#include <condition_variable>
#include <thread>
Expand Down Expand Up @@ -86,13 +85,8 @@ class SHAMapStoreImp : public SHAMapStore
static std::uint32_t const minimumDeletionInterval_ = 256;
// minimum # of ledgers required for standalone mode.
static std::uint32_t const minimumDeletionIntervalSA_ = 8;
// Ledger sequence at which the last deletion interval was triggered,
// or the current validated sequence as of first use
// if there have been no prior deletions. Deletion occurs up to (but
// not including) this value. All ledgers past this value are accumulated
// until the next online deletion. This value is persisted to SQLite
// nearly immediately after modification.
std::atomic<LedgerIndex> lastRotated_{};
// minimum ledger to maintain online.
std::atomic<LedgerIndex> minimumOnline_{};

NodeStore::Scheduler& scheduler_;
beast::Journal const journal_;
Expand Down Expand Up @@ -168,7 +162,7 @@ class SHAMapStoreImp : public SHAMapStore
LedgerIndex
getLastRotated() override
{
return lastRotated_;
return state_db_.getState().lastRotated;
}

// All ledgers before and including this are unprotected
Expand All @@ -187,6 +181,9 @@ class SHAMapStoreImp : public SHAMapStore
int
fdRequired() const override;

boost::optional<LedgerIndex>
minimumOnline() const override;

private:
// callback for visitNodes
bool
Expand Down

0 comments on commit 17412d1

Please sign in to comment.