Skip to content

feat(p2p): add ability to update peer_id.json with SIGUSR1 #981

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

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

glevco
Copy link
Contributor

@glevco glevco commented Mar 21, 2024

Motivation

Implement ability to update the full node public IP while the sync is running, as requested in the issue: resolves https://github.com/HathorNetwork/internal-issues/issues/261.

Here are the new ways of interacting with entrypoints and the hostname:

Sysctl hostname command

Get and manually set the full node's hostname. Entrypoints will automatically reflect this change, and connections will be reset.

Sysctl refresh_auto_hostname command

Automatically set the hostname to an auto discovered one. Entrypoints will automatically reflect this change, and connections will be reset.

Sysctl reload_entrypoints_and_connections command

Reload the peer_id.json file, replacing the entrypoints list with the new one. Then, connections will be reset.

SIGUSR1 command

This is just a shortcut to the sysctl reload_entrypoints_and_connections command.

Acceptance Criteria

  • Remove CliBuilder.create_peer_id() and add PeerId.create_from_json_path().
  • Refactor ConnectionsManager.listen() so it uses the listen deferred and persists the listened addresses.
  • Implement ConnectionsManager.update_hostname_entrypoints() and reset_all_connections().
  • Move peer_id.json file path to the PeerId class, with the reload_entrypoints_from_source_file() method.
  • Add sysctl commands for getting/setting hostname, refreshing the auto hostname, and resetting all connections.
  • Add peer_id.json reload in SIGUSR1.

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 Mar 21, 2024
@glevco glevco force-pushed the feat/p2p/update-peer-id branch from 32beb29 to 968b96c Compare March 21, 2024 22:02
Copy link

codecov bot commented Mar 21, 2024

Codecov Report

Attention: Patch coverage is 43.75000% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 85.10%. Comparing base (23f764d) to head (adc0191).

❗ Current head adc0191 differs from pull request most recent head b9abf07. Consider uploading reports for the commit b9abf07 to get more accurate results

Files Patch % Lines
hathor/manager.py 30.00% 7 Missing ⚠️
hathor/p2p/peer_id.py 60.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #981   +/-   ##
=======================================
  Coverage   85.10%   85.10%           
=======================================
  Files         296      296           
  Lines       22902    22909    +7     
  Branches     3447     3449    +2     
=======================================
+ Hits        19490    19497    +7     
- Misses       2728     2732    +4     
+ Partials      684      680    -4     

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

@glevco glevco force-pushed the feat/p2p/update-peer-id branch 2 times, most recently from 26f53bc to 3fd7a4d Compare March 25, 2024 17:30
@glevco glevco marked this pull request as ready for review March 25, 2024 17:38
jansegre
jansegre previously approved these changes Apr 2, 2024
@jansegre jansegre mentioned this pull request Apr 5, 2024
2 tasks
@glevco glevco force-pushed the feat/p2p/update-peer-id branch from cccfe88 to adc0191 Compare April 9, 2024 19:32
@glevco glevco force-pushed the feat/p2p/update-peer-id branch 5 times, most recently from df90865 to 83e2aca Compare April 10, 2024 15:31
@glevco glevco requested review from jansegre and msbrogli April 10, 2024 15:36
@glevco
Copy link
Contributor Author

glevco commented Apr 10, 2024

@msbrogli @jansegre This PR changed considerably, could you please re-review it?

The description has been updated to reflect new changes. Also, I ended up not implementing the looping call to refresh the auto hostname, and instead just provided a sysctl command to do it. The reason is that the API is currently blocking.

jansegre
jansegre previously approved these changes Apr 10, 2024
msbrogli
msbrogli previously approved these changes Apr 16, 2024
@glevco glevco dismissed stale reviews from msbrogli and jansegre via 647f423 April 16, 2024 20:56
jansegre
jansegre previously approved these changes Apr 16, 2024
msbrogli
msbrogli previously approved these changes Apr 16, 2024
@glevco glevco dismissed stale reviews from msbrogli and jansegre via ea94b70 April 16, 2024 21:39
@glevco glevco force-pushed the feat/p2p/update-peer-id branch 2 times, most recently from ea94b70 to e91f983 Compare April 16, 2024 22:53
@glevco glevco force-pushed the feat/p2p/update-peer-id branch from e91f983 to b9abf07 Compare April 16, 2024 22:55
@jansegre jansegre merged commit 39a22fe into master Apr 17, 2024
11 checks passed
@jansegre jansegre deleted the feat/p2p/update-peer-id branch April 17, 2024 00:09
@jansegre jansegre mentioned this pull request May 8, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants