Skip to content
This repository has been archived by the owner on Oct 31, 2024. It is now read-only.

feat: add p2p layer health check #464

Merged
merged 5 commits into from
Mar 12, 2024
Merged

feat: add p2p layer health check #464

merged 5 commits into from
Mar 12, 2024

Conversation

Freyskeyd
Copy link
Member

@Freyskeyd Freyskeyd commented Feb 27, 2024

Description

This PR is adding and refactoring how the node will setup and validate it's p2p layer connectivity.
It is a first step to allow nodes to be spawn in any order and gain more confidence on the node status.

Changes

  • discovery behaviour will periodically walk the network to find new peers and discover more nodes.
    • The default setting is currently fixed to 60s but will be configurable in another PR
    • The first query is targeting one bootnode, if the query fails, the interval between bootstrap query is set to 5s (also configurable later)
  • gossip behaviour will subscribe to different topics and become healthy when at least one is ok
  • the handle_event method of the P2P runtime is returning a Result which allows us to report error (and maybe crash) if needed.
  • StateMachine in P2P layer represents a struct that hold state projection (to avoid extra calculation), it may be refactored to enum and better integrated with the events but for now it is enough.

Steps

  • The P2P layer is starting by listening and exposing its addresses.
  • Then the discovery behaviour will initiate bootstrap query targeting the bootnode.
    The connection and result of this query will define if we go or not to the next step.
  • The gossipsub behaviour will then trigger the subscribe to the topics
  • When succeed the P2P layer is considered healthy and can continue

PR Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added or updated tests that comprehensively prove my change is effective or that my feature works

@Freyskeyd Freyskeyd force-pushed the refactor/store-instantiation branch from 6166fda to 9f81725 Compare March 1, 2024 20:57
@Freyskeyd Freyskeyd force-pushed the feature/tec-23 branch 2 times, most recently from 93d26bc to 556d295 Compare March 1, 2024 21:34
@Freyskeyd Freyskeyd force-pushed the refactor/store-instantiation branch from 9f81725 to 9e27632 Compare March 5, 2024 09:54
Base automatically changed from refactor/store-instantiation to main March 5, 2024 17:22
@Freyskeyd Freyskeyd force-pushed the feature/tec-23 branch 2 times, most recently from 01afb73 to 7d06abf Compare March 6, 2024 09:49
Copy link

codecov bot commented Mar 6, 2024

Codecov Report

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

Project coverage is 71.60%. Comparing base (264c569) to head (69601cf).

Files Patch % Lines
crates/topos-tce/src/app_context/network.rs 50.42% 58 Missing ⚠️
crates/topos-tce/src/lib.rs 14.81% 23 Missing ⚠️
crates/topos-p2p/src/behaviour/gossip.rs 85.71% 10 Missing ⚠️
crates/topos-p2p/src/runtime/handle_event.rs 86.53% 7 Missing ⚠️
crates/topos-p2p/src/runtime/mod.rs 81.08% 7 Missing ⚠️
...es/topos-p2p/src/runtime/handle_event/discovery.rs 94.66% 4 Missing ⚠️
crates/topos-test-sdk/src/tce/mod.rs 90.00% 2 Missing ⚠️
crates/topos-p2p/src/event.rs 83.33% 1 Missing ⚠️
crates/topos-test-sdk/src/tce/p2p.rs 92.30% 1 Missing ⚠️
crates/topos-test-sdk/src/tce/protocol.rs 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #464      +/-   ##
==========================================
+ Coverage   70.70%   71.60%   +0.89%     
==========================================
  Files         225      225              
  Lines       12410    12504      +94     
==========================================
+ Hits         8775     8953     +178     
+ Misses       3635     3551      -84     

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

@Freyskeyd Freyskeyd force-pushed the feature/tec-23 branch 4 times, most recently from 99a6d84 to 47fa328 Compare March 6, 2024 20:49
@Freyskeyd Freyskeyd marked this pull request as ready for review March 6, 2024 21:11
@Freyskeyd Freyskeyd requested a review from a team as a code owner March 6, 2024 21:11
@Freyskeyd Freyskeyd requested review from dvdplm and JDawg287 March 6, 2024 21:11
.github/workflows/docker_build_push.yml Outdated Show resolved Hide resolved
.github/workflows/docker_build_push.yml Outdated Show resolved Hide resolved
crates/topos-config/src/edge/command.rs Show resolved Hide resolved
crates/topos-p2p/src/behaviour/discovery.rs Outdated Show resolved Hide resolved
crates/topos-p2p/src/behaviour/discovery.rs Show resolved Hide resolved
crates/topos-p2p/src/config.rs Show resolved Hide resolved
crates/topos-p2p/src/config.rs Show resolved Hide resolved
crates/topos-p2p/src/runtime/handle_event/discovery.rs Outdated Show resolved Hide resolved
crates/topos-p2p/src/runtime/handle_event/discovery.rs Outdated Show resolved Hide resolved
crates/topos-tce-api/src/runtime/sync_task.rs Outdated Show resolved Hide resolved
Copy link
Member

@hadjiszs hadjiszs left a comment

Choose a reason for hiding this comment

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

Also saw several occurrences "bootpeers" and "bootnodes" maybe let's try to stick to one or you wanna actually keep distinction for some reason?

crates/topos-p2p/src/behaviour/gossip.rs Outdated Show resolved Hide resolved
crates/topos-config/src/edge/command.rs Show resolved Hide resolved
crates/topos-p2p/src/config.rs Outdated Show resolved Hide resolved
crates/topos-p2p/src/runtime/mod.rs Show resolved Hide resolved
crates/topos-p2p/src/runtime/mod.rs Outdated Show resolved Hide resolved
Signed-off-by: Simon Paitrault <simon.paitrault@gmail.com>
Signed-off-by: Simon Paitrault <simon.paitrault@gmail.com>
Signed-off-by: Simon Paitrault <simon.paitrault@gmail.com>
Signed-off-by: Simon Paitrault <simon.paitrault@gmail.com>
Signed-off-by: Simon Paitrault <simon.paitrault@gmail.com>
@Freyskeyd Freyskeyd merged commit d2ec941 into main Mar 12, 2024
21 checks passed
@Freyskeyd Freyskeyd deleted the feature/tec-23 branch March 12, 2024 16:07
@Freyskeyd Freyskeyd added release and removed release labels Mar 19, 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