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(vertex-handler): remove p2p_manager dependency [part 1/8] #1154

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

glevco
Copy link
Contributor

@glevco glevco commented Oct 10, 2024

Depends on #1153

Motivation

As part of the P2P Multiprocess project, this PR refactors the VertexHandler so it does not depend on the p2p_manager anymore.

Acceptance Criteria

  • Remove the p2p_manager dependency from VertexHandler. This means it does not propagate vertices anymore, and it's the caller's responsibility to do it.
  • Remove duplicate partial_vertex_exists methods from sync classes and move it to the storage.
  • Update all calls to VertexHandler.on_new_vertex to propagate vertices accordingly:
    • If the call explicitly set propagate_to_peers=False, no change was necessary.
    • If the call didn't provide the propagate_to_peers argument or set it as True, add a follow-up call to p2p_manager.send_tx_to_peers.
  • Move CPU profiler from HathorManager.on_new_tx to VertexHandler.on_new_vertex.
  • Fix CPU profiler typing.
  • No changes in behavior.

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

@glevco glevco self-assigned this Oct 10, 2024
@glevco glevco marked this pull request as ready for review October 10, 2024 16:10
@glevco glevco force-pushed the refactor/vertex-handler/remove-p2p-manager branch from 10cad94 to 4da7ddb Compare October 10, 2024 16:56
Copy link

codecov bot commented Oct 10, 2024

Codecov Report

Attention: Patch coverage is 93.93939% with 2 lines in your changes missing coverage. Please review.

Project coverage is 84.59%. Comparing base (2536592) to head (3203fbc).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
hathor/p2p/sync_v2/agent.py 85.71% 0 Missing and 1 partial ⚠️
hathor/p2p/sync_v2/mempool.py 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1154      +/-   ##
==========================================
- Coverage   84.66%   84.59%   -0.07%     
==========================================
  Files         313      313              
  Lines       24245    24249       +4     
  Branches     3687     3690       +3     
==========================================
- Hits        20527    20514      -13     
- Misses       3004     3016      +12     
- Partials      714      719       +5     

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

@glevco glevco force-pushed the refactor/vertex-handler/remove-p2p-manager branch 2 times, most recently from bb13031 to 3c767f7 Compare October 10, 2024 20:34
@glevco glevco changed the title refactor(vertex-handler): remove p2p_manager dependency refactor(vertex-handler): remove p2p_manager dependency [part 3/5] Oct 10, 2024
@glevco glevco force-pushed the refactor/move-can-validate-full branch from b6430fe to 409f3f3 Compare October 14, 2024 19:12
@glevco glevco force-pushed the refactor/vertex-handler/remove-p2p-manager branch from 3c767f7 to dc2977f Compare October 15, 2024 14:56
@glevco glevco changed the title refactor(vertex-handler): remove p2p_manager dependency [part 3/5] refactor(vertex-handler): remove p2p_manager dependency [part 3/7] Oct 15, 2024
@glevco glevco changed the title refactor(vertex-handler): remove p2p_manager dependency [part 3/7] refactor(vertex-handler): remove p2p_manager dependency [part 3/8] Oct 15, 2024
@glevco glevco force-pushed the refactor/move-can-validate-full branch 2 times, most recently from 8e980c1 to 7dd0b6d Compare October 17, 2024 18:01
@glevco glevco force-pushed the refactor/vertex-handler/remove-p2p-manager branch from dc2977f to 9d98daf Compare October 17, 2024 18:02
Base automatically changed from refactor/move-can-validate-full to master October 18, 2024 19:14
@glevco glevco force-pushed the refactor/vertex-handler/remove-p2p-manager branch 2 times, most recently from 36692d0 to b35cea4 Compare October 18, 2024 19:22
Copy link

github-actions bot commented Oct 18, 2024

🐰 Bencher Report

Branchrefactor/vertex-handler/remove-p2p-manager
Testbedubuntu-22.04
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
nanoseconds (ns)
(Result Δ%)
Lower Boundary
nanoseconds (ns)
(Limit %)
Upper Boundary
nanoseconds (ns)
(Limit %)
sync-v2 (up to 20000 blocks)📈 view plot
🚷 view threshold
101,502,751,945.94
(-0.32%)
91,650,003,337.89
(90.29%)
112,016,670,746.31
(90.61%)
🐰 View full continuous benchmarking report in Bencher

@glevco glevco force-pushed the refactor/vertex-handler/remove-p2p-manager branch from b35cea4 to 14e43d1 Compare October 22, 2024 14:46
@glevco glevco changed the title refactor(vertex-handler): remove p2p_manager dependency [part 3/8] refactor(vertex-handler): remove p2p_manager dependency [part 1/8] Oct 23, 2024
hathor/manager.py Show resolved Hide resolved
@glevco glevco force-pushed the refactor/vertex-handler/remove-p2p-manager branch from 14e43d1 to 3203fbc Compare October 24, 2024 20:41
@glevco glevco merged commit 123166e into master Oct 25, 2024
11 of 13 checks passed
@glevco glevco deleted the refactor/vertex-handler/remove-p2p-manager branch October 25, 2024 13:18
@jansegre jansegre mentioned this pull request Nov 21, 2024
2 tasks
@jansegre jansegre mentioned this pull request Dec 11, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants