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

feat: add peering support #194

Merged
merged 4 commits into from
Nov 21, 2023
Merged

Conversation

smrz2001
Copy link
Collaborator

This PR adds support for dialing bootstrap peers with exponential backoff in case of connection or dial failures.

Also, somehow, support for injecting bootstrap addresses got removed from the code, so I added it back.

@smrz2001 smrz2001 self-assigned this Nov 20, 2023
Copy link

linear bot commented Nov 20, 2023

WS1-1328 Add peering support

This will be a counterpart to the Kubo peering implementation so that specific peers can stay reliably swarm connected.

libp2p_swarm has nice testing APIs and the ability to mock time via tokio.

DOD:

  • Rust peers with the bootstrap list, meaning it will continually dial any peers in the bootstrap list that are disconnected
  • Dials will have an exponential back off if they are unreachable, max delay of 5m

@smrz2001
Copy link
Collaborator Author

Looks like libp2p has deprecated some stuff and Clippy is complaining:

error: use of deprecated associated type `libp2p::libp2p_swarm::ConnectionHandler::Error`: Will be removed together with `ConnectionHandlerEvent::Close`. See <https://github.com/libp2p/rust-libp2p/issues/3591> for details.
   --> recon/src/libp2p/handler.rs:205:13
    |
205 |             Self::Error,
    |             ^^^^^^^^^^^
    |
    = note: `-D deprecated` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(deprecated)]`

    Checking libp2p-quic v0.9.3
error: use of deprecated tuple variant `libp2p::libp2p_swarm::ConnectionHandlerEvent::Close`: To close a connection, use `ToSwarm::CloseConnection` or `Swarm::close_connection`. See <https://github.com/libp2p/rust-libp2p/issues/3591> for more details.
   --> beetle/iroh-bitswap/src/handler.rs:334:55
    |
334 |                         yield ConnectionHandlerEvent::Close(err);
    |                                                       ^^^^^
    |
    = note: `-D deprecated` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(deprecated)]`

    Checking ceramic-event v0.9.0 (/home/runner/work/rust-ceramic/rust-ceramic/event)
    Checking iroh-rpc-client v0.9.0 (/home/runner/work/rust-ceramic/rust-ceramic/beetle/iroh-rpc-client)
    ```

metrics/src/p2p.rs Outdated Show resolved Hide resolved
@smrz2001 smrz2001 force-pushed the feature/ws1-1328-add-peering-support branch from ffa5fd1 to 8ad0ab9 Compare November 20, 2023 00:21
one/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@nathanielc nathanielc left a comment

Choose a reason for hiding this comment

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

Looks great, gave it a complete review now. Just a few small recommendations.

one/src/lib.rs Show resolved Hide resolved
one/src/lib.rs Outdated Show resolved Hide resolved
p2p/src/behaviour/peer_manager.rs Show resolved Hide resolved
p2p/src/behaviour/peer_manager.rs Outdated Show resolved Hide resolved
p2p/src/behaviour/peer_manager.rs Outdated Show resolved Hide resolved
p2p/src/behaviour/peer_manager.rs Outdated Show resolved Hide resolved
p2p/src/behaviour/peer_manager.rs Show resolved Hide resolved
p2p/src/node.rs Show resolved Hide resolved
@smrz2001 smrz2001 force-pushed the feature/ws1-1328-add-peering-support branch from 4515681 to 598664b Compare November 21, 2023 17:23
Copy link
Collaborator

@nathanielc nathanielc left a comment

Choose a reason for hiding this comment

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

:shipit:

@smrz2001 smrz2001 added this pull request to the merge queue Nov 21, 2023
Merged via the queue into main with commit 1e9cba6 Nov 21, 2023
4 checks passed
@smrz2001 smrz2001 deleted the feature/ws1-1328-add-peering-support branch November 21, 2023 18:05
long,
use_value_delimiter = true,
value_delimiter = ',',
env = "CERAMIC_ONE_EXTRA_BOOTSTRAP_ADDRESSES"
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we call these "bootstrap" addresses or something like "peering" addresses? We're starting to have more people ask about running ComposeDB with a high availability setup. For one user to run multiple nodes in an HA cluster, they'll need to make sure all of those nodes are well connected, so I imagine we'll be starting to tell people to use this option soon, so want to be aligned on the best terminology here.

I feel like the hard coded ones are "bootstrap" addresses because they're the main way you discover the rest of the network, but if you're manually setting other addresses to connect to, that's more like a "peering" address (which I think is also the terminology that kubo uses?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Rust-ceramic doesn't distinguish between bootstrap vs peering. I don't have a preference for which term we use, however we should only use one.

Copy link
Collaborator Author

@smrz2001 smrz2001 Nov 21, 2023

Choose a reason for hiding this comment

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

I'm also cool either way, if you have a preference for marking them all either "bootstrap" or "peering", @stbrody.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, if we feel like we want to keep the same term for both the hard-coded and user-provided peers then I don't feel strongly which one we use. Fine to keep things as-is

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants