refactor(telemetry): use ranges algorithms in ValidationTracker, mark accessors nodiscard

Nine std::count_if calls over window1h_ and window7d_ took an iterator
pair; std::ranges::count_if takes the container. One hand-rolled
erase-while-iterating loop becomes a single std::erase_if: pending_ is a
hash_map, which is a std::unordered_map alias, so the C++20 overload applies.

The hard-trim loop below it is left as a loop on purpose. It re-tests
pending_.size() every step to stop as soon as the cap is met, which erase_if
cannot express.

The accessors return values a caller must use: 13 in ValidationTracker.h, the
two Unit.h mappers, the two HistogramBuckets.h helpers, getMeter() and
networkTypeFromId(). No caller in the chain discards any of them.
This commit is contained in:
Pratik Mankawde
2026-09-03 15:01:21 +01:00
parent d687903c3f
commit 90bc58a658
5 changed files with 35 additions and 43 deletions

View File

@@ -53,7 +53,7 @@ enum class Unit : std::uint8_t {
* @param unit The unit to translate.
* @return A static, null-terminated UCUM code.
*/
constexpr char const*
[[nodiscard]] constexpr char const*
otelUnitCode(Unit unit) noexcept
{
switch (unit)
@@ -78,7 +78,7 @@ otelUnitCode(Unit unit) noexcept
* @param unit The unit to describe.
* @return A static, null-terminated description.
*/
constexpr char const*
[[nodiscard]] constexpr char const*
otelUnitDescription(Unit unit) noexcept
{
switch (unit)

View File

@@ -144,7 +144,7 @@ inline constexpr std::array kByteBuckets{
* @return true when the ladder is non-empty, starts at or above zero, and
* every later edge is strictly greater than its predecessor.
*/
constexpr bool
[[nodiscard]] constexpr bool
isAscendingNonNegative(std::span<double const> ladder) noexcept
{
if (ladder.empty() || ladder.front() < 0.0)
@@ -167,7 +167,7 @@ static_assert(isAscendingNonNegative(kByteBuckets));
* @param ladder Bucket upper bounds.
* @return A vector holding the same edges in the same order.
*/
inline std::vector<double>
[[nodiscard]] inline std::vector<double>
toVector(std::span<double const> ladder)
{
return std::vector<double>(ladder.begin(), ladder.end());

View File

@@ -393,7 +393,7 @@ public:
* @param name Meter name used to identify the instrumentation scope.
* @return A shared pointer to the Meter.
*/
virtual opentelemetry::nostd::shared_ptr<opentelemetry::metrics::Meter>
[[nodiscard]] virtual opentelemetry::nostd::shared_ptr<opentelemetry::metrics::Meter>
getMeter(std::string_view name = kMeterName) = 0;
/**
@@ -486,7 +486,7 @@ makeTelemetrySetup(
* @param networkId The network identifier from [network_id] config.
* @return "mainnet" (0), "testnet" (1), "devnet" (2), or "unknown".
*/
std::string
[[nodiscard]] std::string
networkTypeFromId(std::uint32_t networkId);
} // namespace xrpl::telemetry

View File

@@ -138,21 +138,21 @@ public:
* Agreement percentage over the last 1 hour.
* @return Percentage [0.0, 100.0], or 0.0 if no data.
*/
double
[[nodiscard]] double
agreementPct1h() const;
/**
* Agreement percentage over the last 24 hours.
* @return Percentage [0.0, 100.0], or 0.0 if no data.
*/
double
[[nodiscard]] double
agreementPct24h() const;
/**
* Agreement percentage over the last 7 days.
* @return Percentage [0.0, 100.0], or 0.0 if no data.
*/
double
[[nodiscard]] double
agreementPct7d() const;
/** @} */
@@ -165,37 +165,37 @@ public:
/**
* Number of agreements in the 1-hour window.
*/
uint64_t
[[nodiscard]] uint64_t
agreements1h() const;
/**
* Number of misses in the 1-hour window.
*/
uint64_t
[[nodiscard]] uint64_t
missed1h() const;
/**
* Number of agreements in the 24-hour window.
*/
uint64_t
[[nodiscard]] uint64_t
agreements24h() const;
/**
* Number of misses in the 24-hour window.
*/
uint64_t
[[nodiscard]] uint64_t
missed24h() const;
/**
* Number of agreements in the 7-day window.
*/
uint64_t
[[nodiscard]] uint64_t
agreements7d() const;
/**
* Number of misses in the 7-day window.
*/
uint64_t
[[nodiscard]] uint64_t
missed7d() const;
/** @} */
@@ -208,25 +208,25 @@ public:
/**
* Total agreements since process start.
*/
uint64_t
[[nodiscard]] uint64_t
totalAgreements() const;
/**
* Total misses since process start.
*/
uint64_t
[[nodiscard]] uint64_t
totalMissed() const;
/**
* Total validations this node sent.
*/
uint64_t
[[nodiscard]] uint64_t
totalValidationsSent() const;
/**
* Total network validations observed for comparison.
*/
uint64_t
[[nodiscard]] uint64_t
totalValidationsChecked() const;
/** @} */

View File

@@ -116,19 +116,11 @@ void
ValidationTracker::evictOldPending(TimePoint now)
{
auto const cutoff = now - kLateRepairWindow;
for (auto it = pending_.begin(); it != pending_.end();)
{
if (it->second.reconciled && it->second.recordTime < cutoff)
{
it = pending_.erase(it);
}
else
{
++it;
}
}
std::erase_if(pending_, [cutoff](auto const& entry) {
return entry.second.reconciled && entry.second.recordTime < cutoff;
});
// Hard trim if still over limit. The loop above already removed every
// Hard trim if still over limit. The pass above already removed every
// reconciled entry older than the late-repair window, so here we drop
// any remaining reconciled entry as a last resort.
if (pending_.size() > kMaxPendingEvents)
@@ -155,7 +147,7 @@ ValidationTracker::agreementPct1h() const
if (window1h_.empty())
return 0.0;
auto const agreed = static_cast<double>(
std::count_if(window1h_.begin(), window1h_.end(), [](auto const& e) { return e.agreed; }));
std::ranges::count_if(window1h_, [](auto const& e) { return e.agreed; }));
return (agreed / static_cast<double>(window1h_.size())) * 100.0;
}
@@ -165,8 +157,8 @@ ValidationTracker::agreementPct24h() const
std::scoped_lock const lock(mutex_);
if (window24h_.empty())
return 0.0;
auto const agreed = static_cast<double>(std::count_if(
window24h_.begin(), window24h_.end(), [](auto const& e) { return e.agreed; }));
auto const agreed = static_cast<double>(
std::ranges::count_if(window24h_, [](auto const& e) { return e.agreed; }));
return (agreed / static_cast<double>(window24h_.size())) * 100.0;
}
@@ -175,7 +167,7 @@ ValidationTracker::agreements1h() const
{
std::scoped_lock const lock(mutex_);
return static_cast<uint64_t>(
std::count_if(window1h_.begin(), window1h_.end(), [](auto const& e) { return e.agreed; }));
std::ranges::count_if(window1h_, [](auto const& e) { return e.agreed; }));
}
uint64_t
@@ -183,23 +175,23 @@ ValidationTracker::missed1h() const
{
std::scoped_lock const lock(mutex_);
return static_cast<uint64_t>(
std::count_if(window1h_.begin(), window1h_.end(), [](auto const& e) { return !e.agreed; }));
std::ranges::count_if(window1h_, [](auto const& e) { return !e.agreed; }));
}
uint64_t
ValidationTracker::agreements24h() const
{
std::scoped_lock const lock(mutex_);
return static_cast<uint64_t>(std::count_if(
window24h_.begin(), window24h_.end(), [](auto const& e) { return e.agreed; }));
return static_cast<uint64_t>(
std::ranges::count_if(window24h_, [](auto const& e) { return e.agreed; }));
}
uint64_t
ValidationTracker::missed24h() const
{
std::scoped_lock const lock(mutex_);
return static_cast<uint64_t>(std::count_if(
window24h_.begin(), window24h_.end(), [](auto const& e) { return !e.agreed; }));
return static_cast<uint64_t>(
std::ranges::count_if(window24h_, [](auto const& e) { return !e.agreed; }));
}
double
@@ -209,7 +201,7 @@ ValidationTracker::agreementPct7d() const
if (window7d_.empty())
return 0.0;
auto const agreed = static_cast<double>(
std::count_if(window7d_.begin(), window7d_.end(), [](auto const& e) { return e.agreed; }));
std::ranges::count_if(window7d_, [](auto const& e) { return e.agreed; }));
return (agreed / static_cast<double>(window7d_.size())) * 100.0;
}
@@ -218,7 +210,7 @@ ValidationTracker::agreements7d() const
{
std::scoped_lock const lock(mutex_);
return static_cast<uint64_t>(
std::count_if(window7d_.begin(), window7d_.end(), [](auto const& e) { return e.agreed; }));
std::ranges::count_if(window7d_, [](auto const& e) { return e.agreed; }));
}
uint64_t
@@ -226,7 +218,7 @@ ValidationTracker::missed7d() const
{
std::scoped_lock const lock(mutex_);
return static_cast<uint64_t>(
std::count_if(window7d_.begin(), window7d_.end(), [](auto const& e) { return !e.agreed; }));
std::ranges::count_if(window7d_, [](auto const& e) { return !e.agreed; }));
}
uint64_t