Merge bitcoin/bitcoin#22839: log: improve addrman logging

b65a25a84666d41a0af4ad98ffadfa4ac802d1bb log: improve addrman logging (Martin Zumsande)

Pull request description:

  The addrman helper functions `GetNewBucket()` and `GetTriedBucket()`
  1) log into the wrong category (`BCLog::NET` instead of `BCLog::ADDRMAN`)
  2)  log too unspecifically - especially `GetTriedBucket()` gets called from many  different places (e.g. `Check_()`, `Serialize()`), it seems sufficient to me logging these when moving an address from new to tried. Running a node with `-checkaddrman=1`and net logging currently results in a lot of repetitive log entries.

  This PR moves these log entries to `Add_()` and `Good_()` and also adds logging for `Select_()` (allowing statistics about New/Tried success probabilities), `GetAddr_()`, `ClearNew()` and `MakeTried()`.

ACKs for top commit:
  jnewbery:
    ACK b65a25a84666d41a0af4ad98ffadfa4ac802d1bb
  vasild:
    ACK b65a25a84666d41a0af4ad98ffadfa4ac802d1bb

Tree-SHA512: 90ab0f64eb44b7388a198efccb613577b74989fea73194bda7de8bfbd50bdb19127cb12f5ec645c7859afdb89290614a79e255f3af0a63a58d4f21aa8fe7b696
This commit is contained in:
fanquake 2021-10-21 08:11:24 +08:00
commit 69986de383
No known key found for this signature in database
GPG Key ID: 2EEB9F5CC09526C1
2 changed files with 19 additions and 15 deletions

View File

@ -45,10 +45,7 @@ int AddrInfo::GetTriedBucket(const uint256& nKey, const std::vector<bool>& asmap
{
uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetKey()).GetCheapHash();
uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << GetGroup(asmap) << (hash1 % ADDRMAN_TRIED_BUCKETS_PER_GROUP)).GetCheapHash();
int tried_bucket = hash2 % ADDRMAN_TRIED_BUCKET_COUNT;
uint32_t mapped_as = GetMappedAS(asmap);
LogPrint(BCLog::NET, "IP %s mapped to AS%i belongs to tried bucket %i\n", ToStringIP(), mapped_as, tried_bucket);
return tried_bucket;
return hash2 % ADDRMAN_TRIED_BUCKET_COUNT;
}
int AddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const std::vector<bool>& asmap) const
@ -56,10 +53,7 @@ int AddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const std::
std::vector<unsigned char> vchSourceGroupKey = src.GetGroup(asmap);
uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetGroup(asmap) << vchSourceGroupKey).GetCheapHash();
uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << vchSourceGroupKey << (hash1 % ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP)).GetCheapHash();
int new_bucket = hash2 % ADDRMAN_NEW_BUCKET_COUNT;
uint32_t mapped_as = GetMappedAS(asmap);
LogPrint(BCLog::NET, "IP %s mapped to AS%i belongs to new bucket %i\n", ToStringIP(), mapped_as, new_bucket);
return new_bucket;
return hash2 % ADDRMAN_NEW_BUCKET_COUNT;
}
int AddrInfo::GetBucketPosition(const uint256& nKey, bool fNew, int nBucket) const
@ -481,6 +475,7 @@ void AddrManImpl::ClearNew(int nUBucket, int nUBucketPos)
assert(infoDelete.nRefCount > 0);
infoDelete.nRefCount--;
vvNew[nUBucket][nUBucketPos] = -1;
LogPrint(BCLog::ADDRMAN, "Removed %s from new[%i][%i]\n", infoDelete.ToString(), nUBucket, nUBucketPos);
if (infoDelete.nRefCount == 0) {
Delete(nIdDelete);
}
@ -532,6 +527,8 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId)
infoOld.nRefCount = 1;
vvNew[nUBucket][nUBucketPos] = nIdEvict;
nNew++;
LogPrint(BCLog::ADDRMAN, "Moved %s from tried[%i][%i] to new[%i][%i] to make space\n",
infoOld.ToString(), nKBucket, nKBucketPos, nUBucket, nUBucketPos);
}
assert(vvTried[nKBucket][nKBucketPos] == -1);
@ -582,17 +579,20 @@ void AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nT
// Will moving this address into tried evict another entry?
if (test_before_evict && (vvTried[tried_bucket][tried_bucket_pos] != -1)) {
// Output the entry we'd be colliding with, for debugging purposes
auto colliding_entry = mapInfo.find(vvTried[tried_bucket][tried_bucket_pos]);
LogPrint(BCLog::ADDRMAN, "Collision inserting element into tried table (%s), moving %s to m_tried_collisions=%d\n", colliding_entry != mapInfo.end() ? colliding_entry->second.ToString() : "", addr.ToString(), m_tried_collisions.size());
if (m_tried_collisions.size() < ADDRMAN_SET_TRIED_COLLISION_SIZE) {
m_tried_collisions.insert(nId);
}
// Output the entry we'd be colliding with, for debugging purposes
auto colliding_entry = mapInfo.find(vvTried[tried_bucket][tried_bucket_pos]);
LogPrint(BCLog::ADDRMAN, "Collision with %s while attempting to move %s to tried table. Collisions=%d\n",
colliding_entry != mapInfo.end() ? colliding_entry->second.ToString() : "",
addr.ToString(),
m_tried_collisions.size());
} else {
LogPrint(BCLog::ADDRMAN, "Moving %s to tried\n", addr.ToString());
// move nId to the tried tables
MakeTried(info, nId);
LogPrint(BCLog::ADDRMAN, "Moved %s mapped to AS%i to tried[%i][%i]\n",
addr.ToString(), addr.GetMappedAS(m_asmap), tried_bucket, tried_bucket_pos);
}
}
@ -662,6 +662,8 @@ bool AddrManImpl::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTi
ClearNew(nUBucket, nUBucketPos);
pinfo->nRefCount++;
vvNew[nUBucket][nUBucketPos] = nId;
LogPrint(BCLog::ADDRMAN, "Added %s mapped to AS%i to new[%i][%i]\n",
addr.ToString(), addr.GetMappedAS(m_asmap), nUBucket, nUBucketPos);
} else {
if (pinfo->nRefCount == 0) {
Delete(nId);
@ -720,6 +722,7 @@ std::pair<CAddress, int64_t> AddrManImpl::Select_(bool newOnly) const
assert(it_found != mapInfo.end());
const AddrInfo& info{it_found->second};
if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) {
LogPrint(BCLog::ADDRMAN, "Selected %s from tried\n", info.ToString());
return {info, info.nLastTry};
}
fChanceFactor *= 1.2;
@ -739,6 +742,7 @@ std::pair<CAddress, int64_t> AddrManImpl::Select_(bool newOnly) const
assert(it_found != mapInfo.end());
const AddrInfo& info{it_found->second};
if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) {
LogPrint(BCLog::ADDRMAN, "Selected %s from new\n", info.ToString());
return {info, info.nLastTry};
}
fChanceFactor *= 1.2;
@ -780,7 +784,7 @@ std::vector<CAddress> AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct
addresses.push_back(ai);
}
LogPrint(BCLog::ADDRMAN, "GetAddr returned %d random addresses\n", addresses.size());
return addresses;
}

View File

@ -209,7 +209,7 @@ class InvalidMessagesTest(BitcoinTestFramework):
self.test_addrv2('unrecognized network',
[
'received: addrv2 (25 bytes)',
'IP 9.9.9.9 mapped',
'9.9.9.9:8333 mapped',
'Added 1 addresses',
],
bytes.fromhex(