From 3c7cae49b692bb6bf5cae5ee23479091bed0b8be Mon Sep 17 00:00:00 2001 From: Eugene Siegel Date: Fri, 18 Jul 2025 10:18:59 -0400 Subject: [PATCH] log: change LogLimitStats to struct LogRateLimiter::Stats Clean up the noisy LogLimitStats and remove references to the time window. Co-Authored-By: stickies-v --- src/logging.cpp | 15 +++++------ src/logging.h | 52 +++++++++++++------------------------- src/test/logging_tests.cpp | 30 +++++++++++----------- 3 files changed, 40 insertions(+), 57 deletions(-) diff --git a/src/logging.cpp b/src/logging.cpp index a090803652c..befac8a03f8 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -384,10 +384,10 @@ BCLog::LogRateLimiter::Status BCLog::LogRateLimiter::Consume( const std::string& str) { StdLockGuard scoped_lock(m_mutex); - auto& counter{m_source_locations.try_emplace(source_loc, m_max_bytes).first->second}; - Status status{counter.GetDroppedBytes() > 0 ? Status::STILL_SUPPRESSED : Status::UNSUPPRESSED}; + auto& stats{m_source_locations.try_emplace(source_loc, m_max_bytes).first->second}; + Status status{stats.m_dropped_bytes > 0 ? Status::STILL_SUPPRESSED : Status::UNSUPPRESSED}; - if (!counter.Consume(str.size()) && status == Status::UNSUPPRESSED) { + if (!stats.Consume(str.size()) && status == Status::UNSUPPRESSED) { status = Status::NEWLY_SUPPRESSED; m_suppression_active = true; } @@ -549,18 +549,17 @@ void BCLog::LogRateLimiter::Reset() source_locations.swap(m_source_locations); m_suppression_active = false; } - for (const auto& [source_loc, counter] : source_locations) { - uint64_t dropped_bytes{counter.GetDroppedBytes()}; - if (dropped_bytes == 0) continue; + for (const auto& [source_loc, stats] : source_locations) { + if (stats.m_dropped_bytes == 0) continue; LogPrintLevel_( LogFlags::ALL, Level::Info, /*should_ratelimit=*/false, "Restarting logging from %s:%d (%s): %d bytes were dropped during the last %ss.\n", source_loc.file_name(), source_loc.line(), source_loc.function_name(), - dropped_bytes, Ticks(m_reset_window)); + stats.m_dropped_bytes, Ticks(m_reset_window)); } } -bool BCLog::LogLimitStats::Consume(uint64_t bytes) +bool BCLog::LogRateLimiter::Stats::Consume(uint64_t bytes) { if (bytes > m_available_bytes) { m_dropped_bytes += bytes; diff --git a/src/logging.h b/src/logging.h index 106de1f8d30..04e6e0974c4 100644 --- a/src/logging.h +++ b/src/logging.h @@ -107,44 +107,28 @@ namespace BCLog { constexpr uint64_t RATELIMIT_MAX_BYTES{1024 * 1024}; // maximum number of bytes per source location that can be logged within the RATELIMIT_WINDOW constexpr auto RATELIMIT_WINDOW{1h}; // time window after which log ratelimit stats are reset - //! Keeps track of an individual source location and how many available bytes are left for logging from it. - class LogLimitStats - { - private: - //! Remaining bytes in the current window interval. - uint64_t m_available_bytes; - //! Number of bytes that were not consumed within the current window. - uint64_t m_dropped_bytes{0}; - - public: - LogLimitStats(uint64_t max_bytes) : m_available_bytes{max_bytes} {} - //! Consume bytes from the window if enough bytes are available. - //! - //! Returns whether enough bytes were available. - bool Consume(uint64_t bytes); - - uint64_t GetAvailableBytes() const - { - return m_available_bytes; - } - - uint64_t GetDroppedBytes() const - { - return m_dropped_bytes; - } - }; - - /** - * Fixed window rate limiter for logging. - */ + //! Fixed window rate limiter for logging. class LogRateLimiter { + public: + //! Keeps track of an individual source location and how many available bytes are left for logging from it. + struct Stats { + //! Remaining bytes + uint64_t m_available_bytes; + //! Number of bytes that were consumed but didn't fit in the available bytes. + uint64_t m_dropped_bytes{0}; + + Stats(uint64_t max_bytes) : m_available_bytes{max_bytes} {} + //! Updates internal accounting and returns true if enough available_bytes were remaining + bool Consume(uint64_t bytes); + }; + private: mutable StdMutex m_mutex; - //! Counters for each source location that has attempted to log something. - std::unordered_map m_source_locations GUARDED_BY(m_mutex); - //! True if at least one log location is suppressed. Cached view on m_source_locations for performance reasons. + //! Stats for each source location that has attempted to log something. + std::unordered_map m_source_locations GUARDED_BY(m_mutex); + //! Whether any log locations are suppressed. Cached view on m_source_locations for performance reasons. std::atomic m_suppression_active{false}; public: @@ -155,7 +139,7 @@ namespace BCLog { * reset_window interval. * @param max_bytes Maximum number of bytes that can be logged for each source * location. - * @param reset_window Time window after which the byte counters are reset. + * @param reset_window Time window after which the stats are reset. */ LogRateLimiter(SchedulerFunction scheduler_func, uint64_t max_bytes, std::chrono::seconds reset_window); //! Maximum number of bytes logged per location per window. diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp index 62ed2dffebb..dbe13886af1 100644 --- a/src/test/logging_tests.cpp +++ b/src/test/logging_tests.cpp @@ -348,25 +348,25 @@ BOOST_AUTO_TEST_CASE(logging_log_rate_limiter) BOOST_AUTO_TEST_CASE(logging_log_limit_stats) { - BCLog::LogLimitStats counter{BCLog::RATELIMIT_MAX_BYTES}; + BCLog::LogRateLimiter::Stats stats(BCLog::RATELIMIT_MAX_BYTES); - // Check that counter gets initialized correctly. - BOOST_CHECK_EQUAL(counter.GetAvailableBytes(), BCLog::RATELIMIT_MAX_BYTES); - BOOST_CHECK_EQUAL(counter.GetDroppedBytes(), 0ull); + // Check that stats gets initialized correctly. + BOOST_CHECK_EQUAL(stats.m_available_bytes, BCLog::RATELIMIT_MAX_BYTES); + BOOST_CHECK_EQUAL(stats.m_dropped_bytes, uint64_t{0}); - const uint64_t MESSAGE_SIZE{512 * 1024}; - BOOST_CHECK(counter.Consume(MESSAGE_SIZE)); - BOOST_CHECK_EQUAL(counter.GetAvailableBytes(), BCLog::RATELIMIT_MAX_BYTES - MESSAGE_SIZE); - BOOST_CHECK_EQUAL(counter.GetDroppedBytes(), 0ull); + const uint64_t MESSAGE_SIZE{BCLog::RATELIMIT_MAX_BYTES / 2}; + BOOST_CHECK(stats.Consume(MESSAGE_SIZE)); + BOOST_CHECK_EQUAL(stats.m_available_bytes, BCLog::RATELIMIT_MAX_BYTES - MESSAGE_SIZE); + BOOST_CHECK_EQUAL(stats.m_dropped_bytes, uint64_t{0}); - BOOST_CHECK(counter.Consume(MESSAGE_SIZE)); - BOOST_CHECK_EQUAL(counter.GetAvailableBytes(), BCLog::RATELIMIT_MAX_BYTES - MESSAGE_SIZE * 2); - BOOST_CHECK_EQUAL(counter.GetDroppedBytes(), 0ull); + BOOST_CHECK(stats.Consume(MESSAGE_SIZE)); + BOOST_CHECK_EQUAL(stats.m_available_bytes, BCLog::RATELIMIT_MAX_BYTES - MESSAGE_SIZE * 2); + BOOST_CHECK_EQUAL(stats.m_dropped_bytes, uint64_t{0}); - // Consuming more bytes after already having consumed 1MB should fail. - BOOST_CHECK(!counter.Consume(500)); - BOOST_CHECK_EQUAL(counter.GetAvailableBytes(), 0ull); - BOOST_CHECK_EQUAL(counter.GetDroppedBytes(), 500ull); + // Consuming more bytes after already having consumed RATELIMIT_MAX_BYTES should fail. + BOOST_CHECK(!stats.Consume(500)); + BOOST_CHECK_EQUAL(stats.m_available_bytes, uint64_t{0}); + BOOST_CHECK_EQUAL(stats.m_dropped_bytes, uint64_t{500}); } void LogFromLocation(int location, std::string message)