Repository navigation
apollo_network_benchmark: implement RoundRobin broadcasting logic - #11560
Conversation
|
This change is |
|
There hasn't been any activity on this pull request recently, and in order to prioritize active work, it has been marked as stale. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| let now_seconds = seconds_since_epoch(); | ||
| let round_duration_seconds = args.user.round_duration_seconds; | ||
| let num_nodes: u64 = args.runner.bootstrap.len().try_into().unwrap(); | ||
| let current_round = (now_seconds / round_duration_seconds) % num_nodes; |
There was a problem hiding this comment.
Round-robin uses wrong node count source
Medium Severity
should_broadcast_round_robin derives num_nodes from args.runner.bootstrap.len(), but bootstrap represents known bootstrap peers, not the cluster size. In RoundRobin, this can pick broadcasters from the wrong ID range, and when bootstrap is empty it evaluates % 0, causing a runtime panic.
| let now_seconds = seconds_since_epoch(); | ||
| let round_duration_seconds = args.user.round_duration_seconds; | ||
| let num_nodes: u64 = args.runner.bootstrap.len().try_into().unwrap(); | ||
| let current_round = (now_seconds / round_duration_seconds) % num_nodes; |
There was a problem hiding this comment.
Zero round duration triggers panic
Medium Severity
should_broadcast_round_robin divides by args.user.round_duration_seconds without validating it is nonzero. Passing --round-duration-seconds 0 causes a runtime panic in now_seconds / round_duration_seconds, so RoundRobin can crash immediately from valid CLI parsing.
6e7d057 to
ffaea93
Compare
662465b to
5ad7f33
Compare
ffaea93 to
c686f08
Compare
5ad7f33 to
5b95193
Compare
| let now_seconds = seconds_since_epoch(); | ||
| let round_duration_seconds = args.user.round_duration_seconds; | ||
| let num_nodes: u64 = args.runner.bootstrap.len().try_into().unwrap(); | ||
| let current_round = (now_seconds / round_duration_seconds) % num_nodes; |
There was a problem hiding this comment.
Division by zero in round-robin scheduling arithmetic
Medium Severity
should_broadcast_round_robin has two unguarded division operations on u64 values that can be zero. num_nodes is derived from args.runner.bootstrap.len(), which is zero when no --bootstrap flag is provided. round_duration_seconds is a CLI argument that a user could explicitly set to 0. Either case causes a panic. The RoundRobin path in should_broadcast unconditionally returns true, so the send loop starts and calls this function even with an empty bootstrap list.
c686f08 to
a5e0165
Compare
5b95193 to
588daee
Compare
588daee to
53ed810
Compare
53ed810 to
7bcf933
Compare
0edd3bd to
b7e9596
Compare
7bcf933 to
64d4c22
Compare
Note
Medium Risk
Changes message emission semantics in the stress-test loop and introduces time-based round selection with
unwrap()/division that could panic or misbehave under unexpected config (e.g., empty bootstrap list or zero round duration).Overview
Adds RoundRobin broadcasting behavior to
broadcast_network_stress_test_nodeso only the designated node sends during a given time slice based onseconds_since_epoch,round_duration_seconds, and runnerid.send_stress_test_messagesnow gates sends byMode(always send forAllBroadcast/OneBroadcast, conditional forRoundRobin) and emits atrace!log when a message is broadcast, while keeping existing broadcast metrics updates within the send path.Written by Cursor Bugbot for commit 64d4c22. This will update automatically on new commits. Configure here.