merge bitcoin#22872: improve checkaddrman logging with duration in milliseconds

This commit is contained in:
Kittywhiskers Van Gogh 2024-09-03 14:57:48 +00:00
parent 8d22fe9945
commit cdc8321c4d
No known key found for this signature in database
GPG Key ID: 30CD0C065E5C4AAD
5 changed files with 32 additions and 17 deletions

View File

@ -7,6 +7,8 @@
#include <addrman_impl.h> #include <addrman_impl.h>
#include <hash.h> #include <hash.h>
#include <logging.h>
#include <logging/timer.h>
#include <netaddress.h> #include <netaddress.h>
#include <protocol.h> #include <protocol.h>
#include <random.h> #include <random.h>
@ -389,7 +391,7 @@ void AddrManImpl::Unserialize(Stream& s_)
LogPrint(BCLog::ADDRMAN, "addrman lost %i new and %i tried addresses due to collisions or invalid addresses\n", nLostUnk, nLost); LogPrint(BCLog::ADDRMAN, "addrman lost %i new and %i tried addresses due to collisions or invalid addresses\n", nLostUnk, nLost);
} }
const int check_code{ForceCheckAddrman()}; const int check_code{CheckAddrman()};
if (check_code != 0) { if (check_code != 0) {
throw DbInconsistentError(strprintf( throw DbInconsistentError(strprintf(
"Corrupt data. Consistency check failed with code %s", "Corrupt data. Consistency check failed with code %s",
@ -956,18 +958,19 @@ void AddrManImpl::Check() const
if (m_consistency_check_ratio == 0) return; if (m_consistency_check_ratio == 0) return;
if (insecure_rand.randrange(m_consistency_check_ratio) >= 1) return; if (insecure_rand.randrange(m_consistency_check_ratio) >= 1) return;
const int err{ForceCheckAddrman()}; const int err{CheckAddrman()};
if (err) { if (err) {
LogPrintf("ADDRMAN CONSISTENCY CHECK FAILED!!! err=%i\n", err); LogPrintf("ADDRMAN CONSISTENCY CHECK FAILED!!! err=%i\n", err);
assert(false); assert(false);
} }
} }
int AddrManImpl::ForceCheckAddrman() const int AddrManImpl::CheckAddrman() const
{ {
AssertLockHeld(cs); AssertLockHeld(cs);
LogPrint(BCLog::ADDRMAN, "Addrman checks started: new %i, tried %i, total %u\n", nNew, nTried, vRandom.size()); LOG_TIME_MILLIS_WITH_CATEGORY_MSG_ONCE(
strprintf("new %i, tried %i, total %u", nNew, nTried, vRandom.size()), BCLog::ADDRMAN);
std::unordered_set<int> setTried; std::unordered_set<int> setTried;
std::unordered_map<int, int> mapNew; std::unordered_map<int, int> mapNew;
@ -1047,7 +1050,6 @@ int AddrManImpl::ForceCheckAddrman() const
if (nKey.IsNull()) if (nKey.IsNull())
return -16; return -16;
LogPrint(BCLog::ADDRMAN, "Addrman checks completed successfully\n");
return 0; return 0;
} }

View File

@ -6,6 +6,7 @@
#define BITCOIN_ADDRMAN_IMPL_H #define BITCOIN_ADDRMAN_IMPL_H
#include <logging.h> #include <logging.h>
#include <logging/timer.h>
#include <netaddress.h> #include <netaddress.h>
#include <protocol.h> #include <protocol.h>
#include <serialize.h> #include <serialize.h>
@ -269,12 +270,13 @@ private:
std::pair<CAddress, int64_t> SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs); std::pair<CAddress, int64_t> SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs);
//! Consistency check, taking into account m_consistency_check_ratio. Will std::abort if an inconsistency is detected. //! Consistency check, taking into account m_consistency_check_ratio.
//! Will std::abort if an inconsistency is detected.
void Check() const EXCLUSIVE_LOCKS_REQUIRED(cs); void Check() const EXCLUSIVE_LOCKS_REQUIRED(cs);
//! Perform consistency check, regardless of m_consistency_check_ratio. //! Perform consistency check, regardless of m_consistency_check_ratio.
//! @returns an error code or zero. //! @returns an error code or zero.
int ForceCheckAddrman() const EXCLUSIVE_LOCKS_REQUIRED(cs); int CheckAddrman() const EXCLUSIVE_LOCKS_REQUIRED(cs);
}; };
#endif // BITCOIN_ADDRMAN_IMPL_H #endif // BITCOIN_ADDRMAN_IMPL_H

View File

@ -27,10 +27,12 @@ public:
Timer( Timer(
std::string prefix, std::string prefix,
std::string end_msg, std::string end_msg,
BCLog::LogFlags log_category = BCLog::LogFlags::ALL) : BCLog::LogFlags log_category = BCLog::LogFlags::ALL,
bool msg_on_completion = true) :
m_prefix(std::move(prefix)), m_prefix(std::move(prefix)),
m_title(std::move(end_msg)), m_title(std::move(end_msg)),
m_log_category(log_category) m_log_category(log_category),
m_message_on_completion(msg_on_completion)
{ {
this->Log(strprintf("%s started", m_title)); this->Log(strprintf("%s started", m_title));
m_start_t = GetTime<std::chrono::microseconds>(); m_start_t = GetTime<std::chrono::microseconds>();
@ -38,7 +40,11 @@ public:
~Timer() ~Timer()
{ {
if (m_message_on_completion) {
this->Log(strprintf("%s completed", m_title)); this->Log(strprintf("%s completed", m_title));
} else {
this->Log("completed");
}
} }
void Log(const std::string& msg) void Log(const std::string& msg)
@ -74,14 +80,17 @@ private:
std::chrono::microseconds m_start_t{}; std::chrono::microseconds m_start_t{};
//! Log prefix; usually the name of the function this was created in. //! Log prefix; usually the name of the function this was created in.
const std::string m_prefix{}; const std::string m_prefix;
//! A descriptive message of what is being timed. //! A descriptive message of what is being timed.
const std::string m_title{}; const std::string m_title;
//! Forwarded on to LogPrint if specified - has the effect of only //! Forwarded on to LogPrint if specified - has the effect of only
//! outputting the timing log when a particular debug= category is specified. //! outputting the timing log when a particular debug= category is specified.
const BCLog::LogFlags m_log_category{}; const BCLog::LogFlags m_log_category;
//! Whether to output the message again on completion.
const bool m_message_on_completion;
}; };
} // namespace BCLog } // namespace BCLog
@ -91,6 +100,8 @@ private:
BCLog::Timer<std::chrono::microseconds> UNIQUE_NAME(logging_timer)(__func__, end_msg, log_category) BCLog::Timer<std::chrono::microseconds> UNIQUE_NAME(logging_timer)(__func__, end_msg, log_category)
#define LOG_TIME_MILLIS_WITH_CATEGORY(end_msg, log_category) \ #define LOG_TIME_MILLIS_WITH_CATEGORY(end_msg, log_category) \
BCLog::Timer<std::chrono::milliseconds> UNIQUE_NAME(logging_timer)(__func__, end_msg, log_category) BCLog::Timer<std::chrono::milliseconds> UNIQUE_NAME(logging_timer)(__func__, end_msg, log_category)
#define LOG_TIME_MILLIS_WITH_CATEGORY_MSG_ONCE(end_msg, log_category) \
BCLog::Timer<std::chrono::milliseconds> UNIQUE_NAME(logging_timer)(__func__, end_msg, log_category, /* msg_on_completion=*/false)
#define LOG_TIME_SECONDS(end_msg) \ #define LOG_TIME_SECONDS(end_msg) \
BCLog::Timer<std::chrono::seconds> UNIQUE_NAME(logging_timer)(__func__, end_msg) BCLog::Timer<std::chrono::seconds> UNIQUE_NAME(logging_timer)(__func__, end_msg)

View File

@ -89,8 +89,8 @@ class AsmapTest(BitcoinTestFramework):
self.restart_node(0, ["-asmap", "-checkaddrman=1"]) self.restart_node(0, ["-asmap", "-checkaddrman=1"])
with self.node.assert_debug_log( with self.node.assert_debug_log(
expected_msgs=[ expected_msgs=[
"Addrman checks started: new 1, tried 1, total 2", "CheckAddrman: new 1, tried 1, total 2 started",
"Addrman checks completed successfully", "CheckAddrman: completed",
] ]
): ):
self.node.getnodeaddresses() # getnodeaddresses re-runs the addrman checks self.node.getnodeaddresses() # getnodeaddresses re-runs the addrman checks

View File

@ -280,7 +280,7 @@ class NetTest(DashTestFramework):
self.log.debug("Test that adding a valid address to the tried table succeeds") self.log.debug("Test that adding a valid address to the tried table succeeds")
assert_equal(node.addpeeraddress(address="1.2.3.4", tried=True, port=8333), {"success": True}) assert_equal(node.addpeeraddress(address="1.2.3.4", tried=True, port=8333), {"success": True})
with node.assert_debug_log(expected_msgs=["Addrman checks started: new 0, tried 1, total 1"]): with node.assert_debug_log(expected_msgs=["CheckAddrman: new 0, tried 1, total 1 started"]):
addrs = node.getnodeaddresses(count=0) # getnodeaddresses re-runs the addrman checks addrs = node.getnodeaddresses(count=0) # getnodeaddresses re-runs the addrman checks
assert_equal(len(addrs), 1) assert_equal(len(addrs), 1)
assert_equal(addrs[0]["address"], "1.2.3.4") assert_equal(addrs[0]["address"], "1.2.3.4")
@ -293,7 +293,7 @@ class NetTest(DashTestFramework):
self.log.debug("Test that adding a second address, this time to the new table, succeeds") self.log.debug("Test that adding a second address, this time to the new table, succeeds")
assert_equal(node.addpeeraddress(address="2.0.0.0", port=8333), {"success": True}) assert_equal(node.addpeeraddress(address="2.0.0.0", port=8333), {"success": True})
with node.assert_debug_log(expected_msgs=["Addrman checks started: new 1, tried 1, total 2"]): with node.assert_debug_log(expected_msgs=["CheckAddrman: new 1, tried 1, total 2 started"]):
addrs = node.getnodeaddresses(count=0) # getnodeaddresses re-runs the addrman checks addrs = node.getnodeaddresses(count=0) # getnodeaddresses re-runs the addrman checks
assert_equal(len(addrs), 2) assert_equal(len(addrs), 2)