feat: add per-peer served-bytes top-10 to replication traffic summary#179
Conversation
Follow-up to V2-623. The per-variant traffic counters attribute served bytes to
message types but cannot say which peers pulled them, which is needed to design
per-peer serving budgets and to answer "which peer is repeatedly pulling data"
for the perpetual-bootstrap diagnosis.
Adds per-peer served-bytes accounting for the heavy serve paths (FetchResponse,
SubtreeByteResponse, NeighborSyncResponse) and emits the top 10 peers by bytes
as one additional INFO line on the existing traffic-summary cadence.
- Bounded, sharded served-peers table (16 shards x 16 = 256 peers), a plain
const-constructible static mirroring V2-623's process-global approach
- record_served(): shared read lock + relaxed fetch_add on the fast path (no
global lock per response); one-shard write lock with evict-smallest for new
peers, tracked via a global evictions counter
- log_served_peers_summary(): 32-field INFO line (peer_1..10_{id,bytes,count} +
tracked_peers + evictions), target ant_node::replication::traffic, message
"replication served-peers summary (cumulative)"; peer IDs are full hex to join
against the claiming-bootstrap / pending-cap log lines
- Counting gated on the three named variants at send_replication_response_checked,
the single choke point with both requester PeerId and encoded length
- Emission piggybacks the existing start_traffic_summary_loop tick
Additive only: no wire changes, no behavior changes.
Gates: cargo fmt applied; cargo clippy --all-features clean of the denied
panic/unwrap/expect lints; cargo build --release exit 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds process-global per-peer served-bytes accounting for replication’s heavy response variants and logs a top-10 peer summary on the existing traffic-summary cadence, enabling attribution of replication serving load to specific peers.
Changes:
- Introduces a bounded, sharded per-peer served-bytes table with per-shard eviction and an
evictionscounter. - Records served bytes at the replication response “choke point” for
FetchResponse,SubtreeByteResponse, andNeighborSyncResponse. - Emits a single INFO “served-peers summary” line (top 10 peers + totals) alongside the existing per-variant traffic summary.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/replication/protocol.rs | Adds sharded per-peer served-bytes counters and a top-10 summary log event on the traffic-summary target. |
| src/replication/mod.rs | Hooks per-peer accounting into send_replication_response_checked and adds periodic emission in the traffic summary loop. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub(crate) fn record_served(peer: &PeerId, bytes: usize) { | ||
| let shard = &SERVED_PEERS[served_shard_index(peer)]; | ||
| let bytes = bytes as u64; | ||
|
|
| pub(crate) fn log_served_peers_summary() { | ||
| // Snapshot every shard, then rank. Reads are relaxed and short-lived; a | ||
| // slightly skewed snapshot is fine because values are cumulative and | ||
| // compared as deltas at query time. | ||
| let mut all: Vec<(PeerId, u64, u64)> = Vec::new(); | ||
| for shard in &SERVED_PEERS { |
| /// Number of shards in the served-peers table. Peers are assigned by the low | ||
| /// nibble of their first ID byte; peer IDs are hash-random (BLAKE3), so the | ||
| /// distribution across shards is uniform. Must be a power of two. | ||
| const SERVED_SHARDS: usize = 16; | ||
|
|
||
| /// Max peers tracked per shard. `SERVED_SHARDS * SERVED_SHARD_CAP` = 256 total. | ||
| const SERVED_SHARD_CAP: usize = 16; | ||
|
|
| /// Record `bytes` served to `peer` (V2-684), called from the replication serve | ||
| /// choke point for the heavy response variants. | ||
| /// | ||
| /// Fast path (peer already tracked): a shared read lock on one shard plus two | ||
| /// relaxed `fetch_add`s. Slow path (new peer): a one-shard write lock that | ||
| /// inserts the peer and, if the shard is at capacity, first evicts its | ||
| /// smallest-bytes entry. | ||
| pub(crate) fn record_served(peer: &PeerId, bytes: usize) { |
dirvine
left a comment
There was a problem hiding this comment.
Hermes review — verdict: no severe blockers. The "no functional changes" claim verified against the code.
Head 2be8794, 1 commit. Additive-only: one record_served() call on the serve path plus one extra INFO line per 300s tick. No wire changes, no behavior changes.
Verified (from code, not just the PR body):
- All replication responses flow through
send_replication_response_checked— it's the only call site ofsend_response/send_messagefor replication, and the three counted variants (FetchResponse,SubtreeByteResponse,NeighborSyncResponse) all pass through it withpeer= requester. No bypass paths; double-counting only on genuine re-sends, which are real bandwidth. - Counting point (post-encode) matches V2-623's per-variant table, so per-peer sums reconcile with the existing
*_tx_bytescounters; send failures inflate both identically and relative attribution holds. - Table is bounded (16 shards × 16 = 256 peers, ~tens of KB). Fast path: one sharded shared read lock + two relaxed
fetch_adds. Slow path re-checks under the write lock correctly. Per-shard min-bytes eviction can't practically evict a global top-10 peer; theevictionscounter exposes pressure. PeerIdisCopywithto_bytes()/to_hex()as used;Display== full lowercase hex ==to_hex(), sopeer_N_idjoins cleanly against the existingclaiming bootstraplog lines in ES — the diagnostic goal works as advertised.- No new dependencies (
parking_lot0.12 /saorsa-core0.26.2 already present);Cargo.lockuntouched. 32 fields matches the per-event cap handling established in V2-623.
Caveats (non-blocking):
- "Served" = "encoded for send": a failed send still counts. Consistent with V2-623 and immaterial for identifying heavy pullers.
- Counters are cumulative since process start; rankings converge to all-time heavy peers on long-lived nodes. Rates come from ES deltas, as documented in the code.
CI at review time: Format, Clippy, Docs, Security Audit, Build ×3, Test (no logging) all pass; Test on ubuntu/macos/windows still pending — hold merge until those go green, as Chris already noted.
Approving on the basis above; merge timing remains with the devops lead once tests pass.
Follow-up to V2-623 (V2-684). The per-variant replication traffic counters attribute served bytes to message types but cannot say which peers pulled them — needed to design per-peer serving budgets and to answer "which peer is repeatedly pulling data" for the perpetual-bootstrap diagnosis.
Summary
FetchResponse,SubtreeByteResponse,NeighborSyncResponse— ~99% of served bytes) and emits the top 10 peers by bytes as one additional INFO line on the existing traffic-summary cadence.static, mirroring V2-623's process-global approach (the serve choke point is a free function with no engine handle).record_served(): fast path for already-tracked peers is a shared read lock + relaxedfetch_add(no global lock per response); new peers take a one-shard write lock and evict the shard's smallest-bytes entry when full, tracked via a globalevictionscounter.log_served_peers_summary(): exactly 32 fields (peer_1..10_{id,bytes,count}+tracked_peers+evictions), targetant_node::replication::traffic, messagereplication served-peers summary (cumulative). Peer IDs are full hex (PeerId::to_hex) so they join against the existingclaiming bootstrap/per-source pending caplog lines. Empty slots emit""/0 for a stable ES schema.send_replication_response_checked— the single choke point with both the requesterPeerIdand the encoded byte length in scope.start_traffic_summary_looptick (same 300s cadence).Additive only: no wire changes, no behavior changes.
Notes for reviewers
Test plan
Gates run locally:
cargo fmt --all— appliedcargo clippy --all-features -- -D clippy::panic -D clippy::unwrap_used -D clippy::expect_used— clean (oneclippy::allsort warning fixed withsort_unstable_by_key+Reverse)cargo build --release— exit 0This is a non-functional, observability-only change (additive log line, no wire or behavior changes) and ships straight to production to gather data.
🤖 Generated with Claude Code