AI review feedback

- Remove extraneous function declaration.
- Add a timeout to SHAMapStore_test::testLedgerGaps so it won't hang in
  case of regression.
- Add an optional timeout parameter to SHAMapStore::rendezvous, and
  return a success flag.
  - Use the timeout in SHAMapStore_test::testLedgerGaps to show lack of
    progress in the Store without an arbitrary sleep delay.
- Refactor LedgerMaster::missingFromCompleteLedgerRange to use RangeSet
  functions instead of a naive iteration.
- If lastGoodValidatedLedger_ is default (0) in healthWait(), then act
  as if no ledgers are missing.
This commit is contained in:
Ed Hennis
2026-06-15 20:13:40 -04:00
parent 875bcc530e
commit f5b4ac358f
5 changed files with 32 additions and 30 deletions

View File

@@ -664,9 +664,13 @@ public:
// current ledger from LedgerMaster.
std::this_thread::sleep_for(100ms);
LedgerIndex const deleteSeq = maxSeq;
while (!lm.haveLedger(deleteSeq))
{
std::this_thread::sleep_for(100ms);
std::size_t iterations = 30;
while (!lm.haveLedger(deleteSeq) && --iterations > 0)
{
std::this_thread::sleep_for(100ms);
}
BEAST_EXPECTS(iterations > 25, to_string(iterations));
}
lm.clearLedger(deleteSeq);
@@ -716,7 +720,6 @@ public:
for (int l = 0; l < 5; ++l)
{
env.close();
// DO NOT CALL rendezvous()! You'll end up with a deadlock.
++maxSeq;
// Nothing has changed
BEAST_EXPECTS(
@@ -728,7 +731,9 @@ public:
"Complete Ledgers",
expectedRange(minSeq, deleteSeq, maxSeq),
lm.getCompleteLedgers()));
std::this_thread::sleep_for(1s);
// The Store is "stuck" in healthWait() and won't finish the run() loop until
// it's backfilled
BEAST_EXPECT(!store.rendezvous(100ms));
}
// Put the missing ledger back in LedgerMaster

View File

@@ -1577,23 +1577,14 @@ LedgerMaster::getCompleteLedgers() const
std::size_t
LedgerMaster::missingFromCompleteLedgerRange(LedgerIndex first, LedgerIndex last) const
{
// Make a copy of the range to avoid holding the lock
auto const range = [&] {
RangeSet<LedgerIndex> const target{range(first, last)};
auto const missing = [&target, this] {
std::scoped_lock const sl(completeLock_);
return completeLedgers_;
return target - completeLedgers_;
}();
std::size_t missing = 0;
for (LedgerIndex idx = first; idx <= last; ++idx)
{
if (!boost::icl::contains(range, idx))
{
++missing;
}
}
return missing;
return boost::icl::size(missing);
}
std::optional<NetClock::time_point>

View File

@@ -5,6 +5,7 @@
#include <xrpl/ledger/Ledger.h>
#include <xrpl/nodestore/Manager.h>
#include <chrono>
#include <optional>
namespace xrpl {
@@ -27,8 +28,8 @@ public:
virtual void
start() = 0;
virtual void
rendezvous() const = 0;
virtual bool
rendezvous(std::optional<std::chrono::milliseconds> const& timeout = {}) const = 0;
virtual void
stop() = 0;

View File

@@ -233,14 +233,20 @@ SHAMapStoreImp::onLedgerClosed(std::shared_ptr<Ledger const> const& ledger)
cond_.notify_one();
}
void
SHAMapStoreImp::rendezvous() const
bool
SHAMapStoreImp::rendezvous(std::optional<std::chrono::milliseconds> const& timeout) const
{
if (!working_)
return;
return true;
auto notWorking = [&] { return !working_; };
std::unique_lock<std::mutex> lock(mutex_);
rendezvous_.wait(lock, [&] { return !working_; });
if (timeout)
return rendezvous_.wait_for(lock, *timeout, notWorking);
else
rendezvous_.wait(lock, notWorking);
return true;
}
int
@@ -641,8 +647,9 @@ SHAMapStoreImp::healthWait()
OperatingMode mode = netOPs_->getOperatingMode();
std::unique_lock lock(mutex_);
auto numMissing =
ledgerMaster_->missingFromCompleteLedgerRange(lastGoodValidatedLedger_, index);
auto numMissing = lastGoodValidatedLedger_ == 0
? 0
: ledgerMaster_->missingFromCompleteLedgerRange(lastGoodValidatedLedger_, index);
while (!stop_ && (mode != OperatingMode::FULL || age > ageThreshold_ || numMissing > 0))
{
// this value shouldn't change, so grab it while we have the

View File

@@ -150,8 +150,8 @@ public:
void
onLedgerClosed(std::shared_ptr<Ledger const> const& ledger) override;
void
rendezvous() const override;
bool
rendezvous(std::optional<std::chrono::milliseconds> const& timeout = {}) const override;
int
fdRequired() const override;
@@ -213,8 +213,6 @@ private:
enum class HealthResult { Stopping, KeepGoing };
[[nodiscard]] HealthResult
healthWait();
bool
hasCompleteRange(LedgerIndex first, LedgerIndex last);
public:
void