Skip to content

apollo_network_benchmark: add Sqmr variant to MessageSender with send logic#11551

Open
sirandreww-starkware wants to merge 1 commit into01-08-apollo_network_benchmark_add_sqmr_variant_to_networkprotocol_enumfrom
01-08-apollo_network_benchmark_add_sqmr_variant_to_messagesender_with_send_logic
Open

apollo_network_benchmark: add Sqmr variant to MessageSender with send logic#11551
sirandreww-starkware wants to merge 1 commit into01-08-apollo_network_benchmark_add_sqmr_variant_to_networkprotocol_enumfrom
01-08-apollo_network_benchmark_add_sqmr_variant_to_messagesender_with_send_logic

Conversation

@sirandreww-starkware
Copy link
Contributor

@sirandreww-starkware sirandreww-starkware commented Jan 8, 2026

Note

Low Risk
Low risk: changes are limited to the benchmark stress-test node and only extend MessageSender to support SQMR query sending with basic error logging and response draining.

Overview
Adds an Sqmr variant to the benchmark stress-test node’s MessageSender and wires send_message to issue SQMR queries via SqmrClientSender, spawning a task to drain responses and logging failures.

No SQMR channel registration is implemented yet; NetworkProtocol::Sqmr still hits a todo!() in register_protocol_channels.

Written by Cursor Bugbot for commit 15327ab. This will update automatically on new commits. Configure here.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Contributor Author

sirandreww-starkware commented Jan 8, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@sirandreww-starkware sirandreww-starkware marked this pull request as ready for review January 8, 2026 17:43
@sirandreww-starkware sirandreww-starkware self-assigned this Jan 8, 2026
@github-actions
Copy link

github-actions bot commented Feb 8, 2026

There hasn't been any activity on this pull request recently, and in order to prioritize active work, it has been marked as stale.
This PR will be closed and locked in 7 days if no further activity occurs.
Thank you for your contributions!

@github-actions github-actions bot added the stale label Feb 8, 2026
@github-actions github-actions bot closed this Feb 16, 2026
@sirandreww-starkware sirandreww-starkware force-pushed the 01-08-apollo_network_benchmark_add_sqmr_variant_to_messagesender_with_send_logic branch from 038ce65 to a239b91 Compare February 16, 2026 09:10
@github-actions github-actions bot removed the stale label Feb 17, 2026
@sirandreww-starkware sirandreww-starkware force-pushed the 01-08-apollo_network_benchmark_add_sqmr_variant_to_networkprotocol_enum branch from 28715e4 to f4b4c71 Compare February 19, 2026 07:24
@sirandreww-starkware sirandreww-starkware force-pushed the 01-08-apollo_network_benchmark_add_sqmr_variant_to_messagesender_with_send_logic branch from a239b91 to d31e516 Compare February 19, 2026 07:24
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

}
Err(e) => {
error!("Failed to send SQMR query: {:?}", e);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SQMR silently continues after send failure, inflating metrics

Medium Severity

The Gossipsub branch calls unwrap() on send failure (panicking), while the new Sqmr branch only logs the error and returns normally. Since the caller in send_stress_test_messages unconditionally increments BROADCAST_MESSAGE_COUNT and BROADCAST_MESSAGE_BYTES_SUM after send_message returns, failed SQMR sends are counted as successful, producing misleading benchmark results.

Fix in Cursor Fix in Web

Ok(mut response_manager) => {
tokio::spawn(async move {
while let Some(_response) = response_manager.next().await {}
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unbounded task spawning per SQMR send in stress loop

Medium Severity

Each SQMR send_message call spawns a detached tokio::spawn task to drain responses. Since the caller loops on a heartbeat interval in send_stress_test_messages, this creates an unbounded number of concurrent tasks over time. If responses arrive slower than the send rate, tasks accumulate indefinitely, leading to resource exhaustion — a real risk in a stress test scenario.

Fix in Cursor Fix in Web

@sirandreww-starkware sirandreww-starkware force-pushed the 01-08-apollo_network_benchmark_add_sqmr_variant_to_messagesender_with_send_logic branch from d31e516 to 6f4edea Compare February 19, 2026 08:04
@sirandreww-starkware sirandreww-starkware force-pushed the 01-08-apollo_network_benchmark_add_sqmr_variant_to_networkprotocol_enum branch from f4b4c71 to 98a30f3 Compare February 19, 2026 08:04
@sirandreww-starkware sirandreww-starkware force-pushed the 01-08-apollo_network_benchmark_add_sqmr_variant_to_networkprotocol_enum branch from 98a30f3 to f24b8da Compare March 16, 2026 15:13
@sirandreww-starkware sirandreww-starkware force-pushed the 01-08-apollo_network_benchmark_add_sqmr_variant_to_messagesender_with_send_logic branch from 6f4edea to 15327ab Compare March 16, 2026 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants