narwhal-network: add WaitingPeer #5277
Conversation
Introduce the `NetworkExt` trait for adding extention methods to an `anemo::Network` as well as a `WaitingPeer` type which enables queuing up an rpc to a known but currently disconnected peer.
| async fn do_rpc(self, request: Request<Bytes>) -> Result<Response<Bytes>, BoxError> { | ||
| use tokio::sync::broadcast::error::RecvError; | ||
|
|
||
| let (mut subscriber, _) = self.network.subscribe(); |
There was a problem hiding this comment.
Is it worth moving this after the "already connected" check? Not sure how expensive this is
There was a problem hiding this comment.
The thinking here is that there could be a race between when we do the first "are we connected" check and when we subscribe to the events. This way we're guaranteed to see the event.
| return peer.rpc(request).await.map_err(Into::into); | ||
| } | ||
| } | ||
| Err(RecvError::Closed) => return Err("network is closed".into()), |
There was a problem hiding this comment.
Just curious, can this even happen as long as Self has a clone of anemo::Network here?
There was a problem hiding this comment.
It shouldn't, so maybe i'm being a bit paranoid. But also maybe we can change that to hold a NetworkRef and not a Network in the future if we don't want this type to keep open the network.
| @@ -10,6 +10,7 @@ | |||
| #![allow(clippy::async_yields_async)] | |||
|
|
|||
| pub mod admin; | |||
| pub mod anemo_ext; | |||
There was a problem hiding this comment.
Do you think it's worth moving this to a shared location outside of narwhal, if we'd ever use it in sui code too?
There was a problem hiding this comment.
Yes i do! problem is that "shared" place has in the past been mysten-network and that's in another repo. So what we could probably do is once we move mysten-network into the sui repo then we can do some reshuffling.
Introduce the
NetworkExttrait for adding extension methods to ananemo::Networkas well as aWaitingPeertype which enables queuing up an rpc to a known but currently disconnected peer.Intended to address MystenLabs/anemo#4.