Skip to content

Conversation

@sirandreww-starkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

@sirandreww-starkware sirandreww-starkware force-pushed the 12-08-apollo_network_benchmark_added_infrastructure_for_multiple_network_protocols branch from ee54a68 to 59af4d3 Compare December 9, 2025 12:15
@sirandreww-starkware sirandreww-starkware force-pushed the 12-08-apollo_network_benchmark_added_infrastructure_for_multiple_network_protocols_split branch from 883aea6 to 07a1668 Compare December 9, 2025 12:15
@sirandreww-starkware sirandreww-starkware force-pushed the 12-08-apollo_network_benchmark_added_infrastructure_for_multiple_network_protocols branch from 59af4d3 to 9731b32 Compare December 11, 2025 12:46
@sirandreww-starkware sirandreww-starkware force-pushed the 12-08-apollo_network_benchmark_added_infrastructure_for_multiple_network_protocols_split branch from 07a1668 to a294977 Compare December 11, 2025 12:46
@sirandreww-starkware sirandreww-starkware force-pushed the 12-08-apollo_network_benchmark_added_infrastructure_for_multiple_network_protocols branch from 9731b32 to 21854c2 Compare December 14, 2025 11:30
@sirandreww-starkware sirandreww-starkware force-pushed the 12-08-apollo_network_benchmark_added_infrastructure_for_multiple_network_protocols_split branch 2 times, most recently from 0244e8b to 0f096a6 Compare December 15, 2025 08:51
@sirandreww-starkware sirandreww-starkware force-pushed the 12-08-apollo_network_benchmark_added_infrastructure_for_multiple_network_protocols branch from 21854c2 to 9a27d08 Compare December 15, 2025 08:51
Copy link
Contributor

@noamsp-starkware noamsp-starkware left a comment

Choose a reason for hiding this comment

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

@noamsp-starkware reviewed all commit messages.
Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @sirandreww-starkware)


crates/apollo_network_benchmark/src/bin/broadcast_network_stress_test_node/handlers.rs line 16 at r1 (raw file):

        Ok(duration) => duration.as_secs_f64(),
        Err(_) => {
            let negative_duration = start_time.duration_since(end_time).unwrap();

You can extract the duration from the error returned.


crates/apollo_network_benchmark/src/bin/broadcast_network_stress_test_node/handlers.rs line 16 at r1 (raw file):

        Ok(duration) => duration.as_secs_f64(),
        Err(_) => {
            let negative_duration = start_time.duration_since(end_time).unwrap();

Don't we want to panic here? It seems to me like we could end up with false metrics if we just sweep the problem under the carpet like this.

@sirandreww-starkware sirandreww-starkware force-pushed the 12-08-apollo_network_benchmark_added_infrastructure_for_multiple_network_protocols_split branch from 0f096a6 to 5c03c49 Compare December 22, 2025 09:44
@sirandreww-starkware sirandreww-starkware force-pushed the 12-08-apollo_network_benchmark_added_infrastructure_for_multiple_network_protocols branch from 9a27d08 to 79356bf Compare December 22, 2025 09:44
@sirandreww-starkware sirandreww-starkware force-pushed the 12-08-apollo_network_benchmark_added_infrastructure_for_multiple_network_protocols_split branch from 5c03c49 to ddb831a Compare December 22, 2025 11:11
@sirandreww-starkware sirandreww-starkware force-pushed the 12-08-apollo_network_benchmark_added_infrastructure_for_multiple_network_protocols branch from 79356bf to 92ff2e3 Compare December 22, 2025 11:11
Copy link
Contributor Author

@sirandreww-starkware sirandreww-starkware left a comment

Choose a reason for hiding this comment

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

@sirandreww-starkware made 2 comments.
Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @noamsp-starkware and @sirandreww-starkware).


crates/apollo_network_benchmark/src/bin/broadcast_network_stress_test_node/handlers.rs line 16 at r1 (raw file):

Previously, noamsp-starkware wrote…

You can extract the duration from the error returned.

I went with panicking when this happens instead.


crates/apollo_network_benchmark/src/bin/broadcast_network_stress_test_node/handlers.rs line 16 at r1 (raw file):

Previously, noamsp-starkware wrote…

Don't we want to panic here? It seems to me like we could end up with false metrics if we just sweep the problem under the carpet like this.

Done.

@sirandreww-starkware sirandreww-starkware force-pushed the 12-08-apollo_network_benchmark_added_infrastructure_for_multiple_network_protocols branch from 92ff2e3 to 2bad837 Compare December 23, 2025 11:03
@sirandreww-starkware sirandreww-starkware force-pushed the 12-08-apollo_network_benchmark_added_infrastructure_for_multiple_network_protocols_split branch from ddb831a to 8253ccc Compare December 23, 2025 11:03
@graphite-app graphite-app bot changed the base branch from 12-08-apollo_network_benchmark_added_infrastructure_for_multiple_network_protocols to graphite-base/10625 December 28, 2025 13:39
Copy link
Contributor

@noamsp-starkware noamsp-starkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

@noamsp-starkware reviewed 3 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @sirandreww-starkware).

@sirandreww-starkware sirandreww-starkware force-pushed the 12-08-apollo_network_benchmark_added_infrastructure_for_multiple_network_protocols_split branch from 8253ccc to 5b995ca Compare December 28, 2025 14:25
@graphite-app graphite-app bot changed the base branch from graphite-base/10625 to main December 28, 2025 14:25
@graphite-app
Copy link

graphite-app bot commented Dec 28, 2025

Merge activity

  • Dec 28, 2:25 PM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

@sirandreww-starkware sirandreww-starkware added this pull request to the merge queue Dec 29, 2025
Merged via the queue into main with commit 9375f7d Dec 29, 2025
35 of 62 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 31, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants