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
mtrippled committed Apr 22, 2020
1 parent 020b285 commit 0fff925
Show file tree
Hide file tree
Showing 5 changed files with 72 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 @@ -255,6 +255,10 @@ class LedgerMaster
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
45 changes: 22 additions & 23 deletions src/ripple/app/ledger/impl/LedgerMaster.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
#include <cassert>
#include <limits>
#include <memory>
#include <optional>
#include <vector>

namespace ripple {
Expand Down Expand Up @@ -142,25 +143,14 @@ 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,
std::optional<LedgerIndex> const minimumOnline,
std::uint32_t const candidateLedger,
beast::Journal j)
{
Expand All @@ -174,15 +164,13 @@ 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 online deletion is enabled, whether greater than or equal to
// the minimum ledger to keep online.
if (minimumOnline.has_value())
return candidateLedger >= *minimumOnline;

// Or greater than or equal to the minimum ledger in SQLite, if any.
return minSeq.has_value() && candidateLedger >= *minSeq;
}();

JLOG (j.trace())
Expand Down Expand Up @@ -1903,8 +1891,8 @@ void LedgerMaster::doAdvance (std::unique_lock<std::recursive_mutex>& sl)
"tryAdvance discovered missing " << *missing;
if ((mFillInProgress == 0 || *missing > mFillInProgress) &&
shouldAcquire(mValidLedgerSeq, ledger_history_,
minSqlSeq(app_),
app_.getSHAMapStore().getLastRotated(), *missing,
minSqlSeq(),
app_.getSHAMapStore().minimumOnline(), *missing,
m_journal))
{
JLOG(m_journal.trace()) <<
Expand Down Expand Up @@ -2159,4 +2147,15 @@ 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;
}


} // ripple
6 changes: 6 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,8 @@
#include <ripple/nodestore/Manager.h>
#include <ripple/protocol/ErrorCodes.h>
#include <ripple/core/Stoppable.h>
#include <atomic>
#include <optional>

namespace ripple {

Expand Down Expand Up @@ -66,6 +68,10 @@ 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. */
virtual std::optional<LedgerIndex> const minimumOnline() const = 0;
};

//------------------------------------------------------------------------------
Expand Down
42 changes: 29 additions & 13 deletions src/ripple/app/misc/SHAMapStoreImp.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,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 All @@ -331,7 +331,20 @@ SHAMapStoreImp::run()
ledgerDb_ = &app_.getLedgerDB();

if (advisoryDelete_)
canDelete_ = state_db_.getCanDelete ();
{
canDelete_ = state_db_.getCanDelete();
// On startup, don't acquire ledgers from the network that
// can be deleted.
minimumOnline_ = canDelete_.load() + 1;
}
else
{
// On startup, don't acquire ledgers from the network older than
// the earliest persisted, if any.
auto const minSql = app_.getLedgerMaster().minSqlSeq();
if (minSql.has_value())
minimumOnline_ = *minSql;
}

while (true)
{
Expand All @@ -357,18 +370,18 @@ 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 "
<< " lastRotated " << lastRotated << " deleteInterval "
<< deleteInterval_ << " canDelete_ " << canDelete_;

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

clearPrior (lastRotated_);
clearPrior (lastRotated);
switch (health())
{
case Health::stopping:
Expand Down Expand Up @@ -448,13 +461,13 @@ SHAMapStoreImp::run()

std::string nextArchiveDir =
dbRotating_->getWritableBackend()->getName();
lastRotated_ = validatedSeq;
lastRotated = validatedSeq;
std::shared_ptr<NodeStore::Backend> oldBackend;
{
std::lock_guard lock (dbRotating_->peekMutex());

state_db_.setState (SavedState {newBackend->getName(),
nextArchiveDir, lastRotated_});
nextArchiveDir, lastRotated});
clearCaches (validatedSeq);
oldBackend = dbRotating_->rotateBackends(
std::move(newBackend),
Expand Down Expand Up @@ -598,7 +611,7 @@ SHAMapStoreImp::clearSql (DatabaseCon& database,
min = *m;
}

if(min > lastRotated || health() != Health::ok)
if (min > lastRotated || health() != Health::ok)
return false;

boost::format formattedDeleteQuery (deleteQuery);
Expand Down Expand Up @@ -646,6 +659,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
21 changes: 11 additions & 10 deletions src/ripple/app/misc/SHAMapStoreImp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
#include <ripple/app/ledger/LedgerMaster.h>
#include <ripple/core/DatabaseCon.h>
#include <ripple/nodestore/DatabaseRotating.h>

#include <atomic>
#include <condition_variable>
#include <thread>

Expand Down Expand Up @@ -85,13 +83,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 @@ -167,7 +160,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 @@ -183,6 +176,14 @@ class SHAMapStoreImp : public SHAMapStore
void rendezvous() const override;
int fdRequired() const override;

std::optional<LedgerIndex> const
minimumOnline() const override
{
if (deleteInterval_)
return minimumOnline_;
return {};
}

private:
// callback for visitNodes
bool copyNode (std::uint64_t& nodeCount, SHAMapAbstractNode const &node);
Expand Down

0 comments on commit 0fff925

Please sign in to comment.