Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(consensus): consensus handles its network registrations on its own #409

Merged
merged 1 commit into from
Aug 15, 2024

Conversation

matan-starkware
Copy link
Contributor

@matan-starkware matan-starkware commented Aug 12, 2024

This is because the network registration logic will become more complex with test sync.
In practice this means that after the initial network config we hold off on spawning the task
until all tasks have been set up.


This change is Reviewable

@matan-starkware matan-starkware marked this pull request as ready for review August 12, 2024 10:25
Copy link

codecov bot commented Aug 12, 2024

Codecov Report

Attention: Patch coverage is 28.57143% with 35 lines in your changes missing coverage. Please review.

Project coverage is 76.62%. Comparing base (6e7b138) to head (3ffee06).
Report is 2 commits behind head on main.

Files Patch % Lines
crates/papyrus_node/src/main.rs 27.27% 26 Missing and 6 partials ⚠️
...papyrus_consensus/src/papyrus_consensus_context.rs 40.00% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #409      +/-   ##
==========================================
+ Coverage   76.59%   76.62%   +0.03%     
==========================================
  Files         348      348              
  Lines       36303    36298       -5     
  Branches    36303    36298       -5     
==========================================
+ Hits        27805    27814       +9     
+ Misses       6195     6176      -19     
- Partials     2303     2308       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@matan-starkware matan-starkware force-pushed the matan/consensus/m3/node_main_network_refactor branch from 0a28cb6 to 778ca40 Compare August 12, 2024 10:48
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/rename_config_topic branch from 9f5aa18 to 74a9acc Compare August 12, 2024 13:19
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/node_main_network_refactor branch from 778ca40 to 0039511 Compare August 12, 2024 13:20
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/rename_config_topic branch from 74a9acc to 79c4fee Compare August 12, 2024 13:22
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/node_main_network_refactor branch from 0039511 to 4557ca6 Compare August 12, 2024 13:22
Copy link

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [64.432 ms 64.494 ms 64.566 ms]
change: [-9.9781% -6.4574% -3.3427%] (p = 0.00 < 0.05)
Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
2 (2.00%) high mild
2 (2.00%) high severe

Copy link

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [64.499 ms 64.570 ms 64.656 ms]
change: [-8.3152% -5.0526% -2.2204%] (p = 0.00 < 0.05)
Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
1 (1.00%) high mild
3 (3.00%) high severe

@matan-starkware matan-starkware force-pushed the matan/consensus/m3/rename_config_topic branch from 79c4fee to 91f34db Compare August 12, 2024 13:26
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/node_main_network_refactor branch from 4557ca6 to aa41dfc Compare August 12, 2024 13:26
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/rename_config_topic branch from 91f34db to 9fccc9e Compare August 12, 2024 13:39
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/node_main_network_refactor branch from aa41dfc to 87b40a7 Compare August 12, 2024 13:39
@matan-starkware matan-starkware requested review from eitanm-starkware and removed request for ShahakShama August 12, 2024 13:52
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/rename_config_topic branch from 9fccc9e to 8efbc66 Compare August 13, 2024 10:29
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/node_main_network_refactor branch from 87b40a7 to 999504e Compare August 13, 2024 10:29
Copy link
Contributor

@eitanm-starkware eitanm-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:

Reviewed 1 of 2 files at r1, 4 of 4 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @DvirYo-starkware)

@matan-starkware matan-starkware force-pushed the matan/consensus/m3/rename_config_topic branch from 8efbc66 to e8b3e29 Compare August 13, 2024 13:50
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/node_main_network_refactor branch from 999504e to 2bf24d1 Compare August 13, 2024 13:50
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/rename_config_topic branch from e8b3e29 to 08fd054 Compare August 13, 2024 13:52
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/node_main_network_refactor branch from 2bf24d1 to 507ff6e Compare August 13, 2024 13:53
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/node_main_network_refactor branch from 18d710c to cd909f6 Compare August 14, 2024 10:35
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/rename_config_topic branch from 1ba756f to 25e5eea Compare August 14, 2024 12:19
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/node_main_network_refactor branch from cd909f6 to 2cec2b7 Compare August 14, 2024 12:19
Copy link

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [65.971 ms 66.049 ms 66.137 ms]
change: [-9.7531% -6.3829% -3.3594%] (p = 0.00 < 0.05)
Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
6 (6.00%) high mild
4 (4.00%) high severe

@matan-starkware matan-starkware force-pushed the matan/consensus/m3/rename_config_topic branch from 25e5eea to 0d418b1 Compare August 15, 2024 05:56
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/node_main_network_refactor branch from 2cec2b7 to 89307aa Compare August 15, 2024 05:57
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/rename_config_topic branch from 0d418b1 to dc4448a Compare August 15, 2024 06:43
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/node_main_network_refactor branch from 89307aa to fcf74df Compare August 15, 2024 06:44
Copy link

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [66.044 ms 66.097 ms 66.161 ms]
change: [-8.6090% -5.0999% -2.1192%] (p = 0.00 < 0.05)
Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severe

@matan-starkware matan-starkware force-pushed the matan/consensus/m3/rename_config_topic branch 2 times, most recently from 8a9d1a3 to d3d8e34 Compare August 15, 2024 07:01
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/node_main_network_refactor branch 2 times, most recently from bffa577 to 62cf873 Compare August 15, 2024 07:01
Copy link

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [65.426 ms 65.496 ms 65.582 ms]
change: [-10.408% -6.6969% -3.4989%] (p = 0.00 < 0.05)
Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
3 (3.00%) high mild
3 (3.00%) high severe

@matan-starkware matan-starkware changed the base branch from matan/consensus/m3/rename_config_topic to graphite-base/409 August 15, 2024 07:24
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/node_main_network_refactor branch 2 times, most recently from c7bd97d to f8f2723 Compare August 15, 2024 07:24
@matan-starkware matan-starkware changed the base branch from graphite-base/409 to main August 15, 2024 07:24
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/node_main_network_refactor branch from f8f2723 to 0fc07e5 Compare August 15, 2024 07:26
Copy link

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [65.441 ms 65.551 ms 65.710 ms]
change: [-7.4764% -4.0522% -1.2342%] (p = 0.01 < 0.05)
Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
2 (2.00%) high mild
2 (2.00%) high severe

Copy link
Contributor

@DvirYo-starkware DvirYo-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:

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @matan-starkware)

…ts own

This is because the network registration logic will become more complex with test sync.
In practice this means that after the initial network config we hold off on spawning the task
until all tasks have been set up.
@matan-starkware matan-starkware force-pushed the matan/consensus/m3/node_main_network_refactor branch from 0fc07e5 to 3ffee06 Compare August 15, 2024 11:58
Copy link

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [65.432 ms 65.496 ms 65.573 ms]
change: [-8.8429% -5.5854% -2.7301%] (p = 0.00 < 0.05)
Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
3 (3.00%) high mild
3 (3.00%) high severe

Copy link
Contributor

@eitanm-starkware eitanm-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:

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @matan-starkware)

@matan-starkware
Copy link
Contributor Author

matan-starkware commented Aug 15, 2024

Merge activity

@matan-starkware matan-starkware merged commit 7aaa7cc into main Aug 15, 2024
27 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 17, 2024
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.

3 participants