Replace NetworkWorker::poll with NetworkWorker::next_action #5763
Conversation
tomaka
added
A0-please_review
client/network/src/service.rs
Outdated
| impl<B: BlockT + 'static, H: ExHashT> Future for NetworkWorker<B, H> { | ||
| type Output = Result<(), io::Error>; | ||
| impl<B: BlockT + 'static, H: ExHashT> NetworkWorker<B, H> { | ||
| /// Performs one action on the network, then returns. |
There was a problem hiding this comment.
I am not quite sure I follow. If I read the logic below correctly this does not perform a single action, but block until the entire NetworkWorker is done.
Based on the pull request description I am guessing that this comment describes the behavior that we would like to have in the future. If so, would you mind mentioning that in the comment?
|
|
||
| fn poll(mut self: Pin<&mut Self>, cx: &mut std::task::Context) -> Poll<Self::Output> { | ||
| /// Implementation of `next_action`. Note that this is not an implementation of `Future` but |
There was a problem hiding this comment.
As this is an implementation detail now, it could as well just implement future, no?
There was a problem hiding this comment.
I precisely don't want NetworkWorker to implement Future, otherwise it's impossible to rewrite that as an async block.
| @@ -1019,7 +1028,7 @@ impl<B: BlockT + 'static, H: ExHashT> Future for NetworkWorker<B, H> { | |||
| // Process the next message coming from the `NetworkService`. | |||
| let msg = match this.from_worker.poll_next_unpin(cx) { | |||
| Poll::Ready(Some(msg)) => msg, | |||
| Poll::Ready(None) => return Poll::Ready(Ok(())), | |||
| Poll::Ready(None) => return Poll::Ready(()), | |||
There was a problem hiding this comment.
This return Poll::Ready(()) is used to signal not to poll the NetworkWorker again.
You must call this method in a loop.
With this comment from above in mind one would still continue polling the NetworkWorker, right? Am I missing something?
|
Going to close this. I wanted to rush this before the stabilization because it's nicer, but we can do without. |
As a follow-up to that comment: #5704 (comment)
This opens the door for making the code async-awaitable in the future, without changing any behaviour right now.
polkadot companion: paritytech/polkadot#1029