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

disocvery: Rework mechanism to not interfere with network stack #723

Open
3 tasks
xla opened this issue Jul 2, 2021 · 1 comment
Open
3 tasks

disocvery: Rework mechanism to not interfere with network stack #723

xla opened this issue Jul 2, 2021 · 1 comment
Labels
networking fd > 3 protocol Something concerning the core protocol

Comments

@xla
Copy link
Contributor

xla commented Jul 2, 2021

It has been shown that the current discovery design is not ideal and can interfere with the memberships protocol of handling active and passive sets see #629 (comment) #696 (comment)

  • drop current stream based discovery in favour of static set of bootstrap nodes
  • Implement Protocol reconnect Protocol: Reconnect #211
  • design pull based discovery
@xla xla added protocol Something concerning the core protocol networking fd > 3 labels Jul 2, 2021
@kim
Copy link
Contributor

kim commented Jul 19, 2021

The formulation as a stream is not entirely wrong, but somewhat misleading. The consumption of that stream is wrong, though. Consider:

After the initial bootstrapping, new peers are discovered via the membership protocol. In case we run out of reachable peers, we'd like to re-initiate the bootstrapping process (ie. reconnect). For this to work, we would either need to call some sort of reset on the datastructure containing the bootstrap nodes -- or keep using a stream, which, in the case of a static list, would simply cycle. The stream abstraction can also encapsulate dynamic discovery (eg. (m)DNS) more naturally (it wouldn't need to be Sync, I think).

There currently is a trait Discovery, which we never actually make useful use of:

pub trait Discovery {
    type Addr;
    type Stream: futures::Stream<Item = (PeerId, Vec<Self::Addr>)> + Send;

    fn discover(self) -> Self::Stream;
}

We could reformulate this as:

pub trait Discovery {
    type Addr;
    type Stream: futures::Stream<Item = (PeerId, Vec<Self::Addr>)> + Send;

    fn discover(&self) -> Self::Stream;
}

such that the protocol stack calls discover whenever it wants a (lazy) list of peers to connect to.

I'm not sure this would make things clearer, though, because ultimately this is isomorphic to just passing the stream.

tl;dr The only required change is to pull from the disco stream only when needed, which allows the Static discovery to become infinitary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
networking fd > 3 protocol Something concerning the core protocol
Projects
None yet
Development

No branches or pull requests

2 participants