Skip to content

Commit 8b89ea1

Browse files
ximinezMark Travis
and
Mark Travis
committed
Optimize when to acquire ledgers from the network.
Particulary avoid acquiring ledgers likely to be produced locally very soon. Derived from XRPLF#4764 Co-authored-by: Mark Travis <mtravis@ripple.com>
1 parent e6ef0fc commit 8b89ea1

File tree

5 files changed

+155
-10
lines changed

5 files changed

+155
-10
lines changed

src/ripple/app/ledger/InboundLedger.h

+20
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,26 @@ class InboundLedger final : public TimeoutCounter,
197197
std::unique_ptr<PeerSet> mPeerSet;
198198
};
199199

200+
inline std::string
201+
to_string(InboundLedger::Reason reason)
202+
{
203+
using enum InboundLedger::Reason;
204+
switch (reason)
205+
{
206+
case HISTORY:
207+
return "HISTORY";
208+
case SHARD:
209+
return "SHARD";
210+
case GENERIC:
211+
return "GENERIC";
212+
case CONSENSUS:
213+
return "CONSENSUS";
214+
default:
215+
assert(false);
216+
return "unknown";
217+
}
218+
}
219+
200220
} // namespace ripple
201221

202222
#endif

src/ripple/app/ledger/impl/InboundLedgers.cpp

+71-6
Original file line numberDiff line numberDiff line change
@@ -69,23 +69,51 @@ class InboundLedgersImp : public InboundLedgers
6969
std::uint32_t seq,
7070
InboundLedger::Reason reason) override
7171
{
72+
std::stringstream ss;
73+
ss << "InboundLedger::acquire: "
74+
<< "Request: " << to_string(hash) << ", " << seq
75+
<< " NeedNetworkLedger: "
76+
<< (app_.getOPs().isNeedNetworkLedger() ? "yes" : "no")
77+
<< " Reason: " << to_string(reason) << " Old rule: ";
78+
if (app_.getOPs().isNeedNetworkLedger() &&
79+
(reason != InboundLedger::Reason::GENERIC) &&
80+
(reason != InboundLedger::Reason::CONSENSUS))
81+
ss << "false";
82+
else
83+
ss << "true";
84+
7285
assert(hash.isNonZero());
7386
assert(
7487
reason != InboundLedger::Reason::SHARD ||
7588
(seq != 0 && app_.getShardStore()));
7689

77-
// probably not the right rule
78-
if (app_.getOPs().isNeedNetworkLedger() &&
79-
(reason != InboundLedger::Reason::GENERIC) &&
80-
(reason != InboundLedger::Reason::CONSENSUS))
81-
return {};
90+
bool const isFull = app_.getOPs().isFull();
91+
bool const fallingBehind = app_.getOPs().isFallingBehind();
92+
LedgerIndex const validSeq =
93+
app_.getLedgerMaster().getValidLedgerIndex();
94+
constexpr std::size_t lagLeeway = 20;
95+
bool const nearFuture =
96+
(seq > validSeq) && (seq < validSeq + lagLeeway);
97+
bool const consensus = reason == InboundLedger::Reason::CONSENSUS;
98+
bool const shouldAcquire =
99+
!(isFull && !fallingBehind && (nearFuture || consensus));
100+
ss << " Evaluating whether to acquire ledger " << hash
101+
<< ". full: " << (isFull ? "true" : "false")
102+
<< ". falling behind: " << (fallingBehind ? "true" : "false")
103+
<< ". ledger sequence " << seq << ". Valid sequence: " << validSeq
104+
<< ". Lag leeway: " << lagLeeway
105+
<< ". request for near future ledger: "
106+
<< (nearFuture ? "true" : "false")
107+
<< ". Consensus: " << (consensus ? "true" : "false")
108+
<< ". Acquiring ledger? " << (shouldAcquire ? "true" : "false");
82109

83110
bool isNew = true;
84111
std::shared_ptr<InboundLedger> inbound;
85112
{
86113
ScopedLockType sl(mLock);
87114
if (stopping_)
88115
{
116+
JLOG(j_.debug()) << "Abort (stopping): " << ss.str();
89117
return {};
90118
}
91119

@@ -111,13 +139,19 @@ class InboundLedgersImp : public InboundLedgers
111139
}
112140

113141
if (inbound->isFailed())
142+
{
143+
JLOG(j_.debug()) << "Abort (failed): " << ss.str();
114144
return {};
145+
}
115146

116147
if (!isNew)
117148
inbound->update(seq);
118149

119150
if (!inbound->isComplete())
151+
{
152+
JLOG(j_.debug()) << "Abort (incomplete): " << ss.str();
120153
return {};
154+
}
121155

122156
if (reason == InboundLedger::Reason::HISTORY)
123157
{
@@ -130,14 +164,45 @@ class InboundLedgersImp : public InboundLedgers
130164
if (!shardStore)
131165
{
132166
JLOG(j_.error())
133-
<< "Acquiring shard with no shard store available";
167+
<< "Acquiring shard with no shard store available"
168+
<< ss.str();
134169
return {};
135170
}
136171
if (inbound->getLedger()->stateMap().family().isShardBacked())
137172
shardStore->setStored(inbound->getLedger());
138173
else
139174
shardStore->storeLedger(inbound->getLedger());
140175
}
176+
177+
/* Acquiring ledgers is somewhat expensive. It requires lots of
178+
* computation and network communication. Avoid it when it's not
179+
* appropriate. Every validation from a peer for a ledger that
180+
* we do not have locally results in a call to this function: even
181+
* if we are moments away from validating the same ledger.
182+
*
183+
* When the following are all true, it is very likely that we will
184+
* soon validate the ledger ourselves. Therefore, avoid acquiring
185+
* ledgers from the network if:
186+
* + Our mode is "full". It is very likely that we will build
187+
* the ledger through the normal consensus process, and
188+
* + Our latest ledger is close to the most recently validated ledger.
189+
* Otherwise, we are likely falling behind the network because
190+
* we have been closing ledgers that have not been validated, and
191+
* + The requested ledger sequence is greater than our validated
192+
* ledger, but not far into the future. Otherwise, it is either a
193+
* request for an historical ledger or, if far into the future,
194+
* likely we're quite behind and will benefit from acquiring it
195+
* from the network.
196+
*/
197+
if (!shouldAcquire)
198+
{
199+
// This check should be before the others because it's cheaper, but
200+
// it's at the end for now to test the effectiveness of the change
201+
JLOG(j_.debug()) << "Abort (rule): " << ss.str();
202+
return {};
203+
}
204+
205+
JLOG(j_.debug()) << "Requesting: " << ss.str();
141206
return inbound->getLedger();
142207
}
143208

src/ripple/app/misc/NetworkOPs.cpp

+60-4
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,8 @@ class NetworkOPsImp final : public NetworkOPs
430430
clearLedgerFetch() override;
431431
Json::Value
432432
getLedgerFetchInfo() override;
433+
bool
434+
isFallingBehind() const override;
433435
std::uint32_t
434436
acceptLedger(
435437
std::optional<std::chrono::milliseconds> consensusDelay) override;
@@ -729,6 +731,7 @@ class NetworkOPsImp final : public NetworkOPs
729731
std::atomic<bool> amendmentBlocked_{false};
730732
std::atomic<bool> amendmentWarned_{false};
731733
std::atomic<bool> unlBlocked_{false};
734+
std::atomic<bool> fallingBehind_{false};
732735

733736
ClosureCounter<void, boost::system::error_code const&> waitHandlerCounter_;
734737
boost::asio::steady_timer heartbeatTimer_;
@@ -1810,22 +1813,69 @@ NetworkOPsImp::beginConsensus(uint256 const& networkClosed)
18101813

18111814
auto closingInfo = m_ledgerMaster.getCurrentLedger()->info();
18121815

1813-
JLOG(m_journal.info()) << "Consensus time for #" << closingInfo.seq
1816+
JLOG(m_journal.info()) << "beginConsensus time for #" << closingInfo.seq
18141817
<< " with LCL " << closingInfo.parentHash;
18151818

1816-
auto prevLedger = m_ledgerMaster.getLedgerByHash(closingInfo.parentHash);
1819+
fallingBehind_ = false;
1820+
if (closingInfo.seq < m_ledgerMaster.getValidLedgerIndex() - 1)
1821+
{
1822+
fallingBehind_ = true;
1823+
JLOG(m_journal.warn())
1824+
<< "beginConsensus Current ledger " << closingInfo.seq
1825+
<< " is at least 2 behind validated "
1826+
<< m_ledgerMaster.getValidLedgerIndex();
1827+
}
1828+
1829+
auto const prevLedger =
1830+
m_ledgerMaster.getLedgerByHash(closingInfo.parentHash);
18171831

18181832
if (!prevLedger)
18191833
{
1834+
fallingBehind_ = true;
18201835
// this shouldn't happen unless we jump ledgers
18211836
if (mMode == OperatingMode::FULL)
18221837
{
1823-
JLOG(m_journal.warn()) << "Don't have LCL, going to tracking";
1838+
JLOG(m_journal.warn())
1839+
<< "beginConsensus Don't have LCL, going to tracking";
18241840
setMode(OperatingMode::TRACKING);
18251841
}
18261842

18271843
return false;
18281844
}
1845+
else if (!m_ledgerMaster.isValidated(*prevLedger))
1846+
{
1847+
// Do not merge this block unless it proves useful.
1848+
auto parentLedger = prevLedger;
1849+
for (; parentLedger && !m_ledgerMaster.isValidated(*parentLedger) &&
1850+
parentLedger->info().seq > closingInfo.seq - 20;
1851+
parentLedger = m_ledgerMaster.getLedgerByHash(
1852+
parentLedger->info().parentHash))
1853+
{
1854+
JLOG(m_journal.debug())
1855+
<< "beginConsensus for " << closingInfo.seq << ". Ledger "
1856+
<< parentLedger->info().seq << " (" << parentLedger->info().hash
1857+
<< ") is not validated.";
1858+
}
1859+
if (parentLedger && m_ledgerMaster.isValidated(*parentLedger))
1860+
{
1861+
JLOG(m_journal.debug())
1862+
<< "beginConsensus for " << closingInfo.seq << ". Ledger "
1863+
<< parentLedger->info().seq << " (" << parentLedger->info().hash
1864+
<< ") IS validated.";
1865+
}
1866+
else
1867+
{
1868+
if (parentLedger)
1869+
JLOG(m_journal.warn())
1870+
<< "beginConsensus for " << closingInfo.seq << ". Previous "
1871+
<< closingInfo.seq - parentLedger->info().seq
1872+
<< " ledgers are not validated";
1873+
else
1874+
JLOG(m_journal.warn())
1875+
<< "beginConsensus for " << closingInfo.seq
1876+
<< ". Ran out of parent ledgers to check.";
1877+
}
1878+
}
18291879

18301880
assert(prevLedger->info().hash == closingInfo.parentHash);
18311881
assert(
@@ -1863,7 +1913,7 @@ NetworkOPsImp::beginConsensus(uint256 const& networkClosed)
18631913
mLastConsensusPhase = currPhase;
18641914
}
18651915

1866-
JLOG(m_journal.debug()) << "Initiating consensus engine";
1916+
JLOG(m_journal.debug()) << "beginConsensus Initiating consensus engine";
18671917
return true;
18681918
}
18691919

@@ -2749,6 +2799,12 @@ NetworkOPsImp::getLedgerFetchInfo()
27492799
return app_.getInboundLedgers().getInfo();
27502800
}
27512801

2802+
bool
2803+
NetworkOPsImp::isFallingBehind() const
2804+
{
2805+
return fallingBehind_;
2806+
}
2807+
27522808
void
27532809
NetworkOPsImp::pubProposedTransaction(
27542810
std::shared_ptr<ReadView const> const& ledger,

src/ripple/app/misc/NetworkOPs.h

+2
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,8 @@ class NetworkOPs : public InfoSub::Source
227227
clearLedgerFetch() = 0;
228228
virtual Json::Value
229229
getLedgerFetchInfo() = 0;
230+
virtual bool
231+
isFallingBehind() const = 0;
230232

231233
/** Accepts the current transaction tree, return the new ledger's sequence
232234

src/ripple/protocol/LedgerHeader.h

+2
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ struct LedgerHeader
5555

5656
// If validated is false, it means "not yet validated."
5757
// Once validated is true, it will never be set false at a later time.
58+
// NOTE: If you are accessing this directly, you are probably doing it
59+
// wrong. Use LedgerMaster::isValidated().
5860
// VFALCO TODO Make this not mutable
5961
bool mutable validated = false;
6062
bool accepted = false;

0 commit comments

Comments
 (0)