Delay beefy worker initialization while network is on major sync #10705
Conversation
Wizdave97
force-pushed
the
david/delay-beefy-on-major-sync
branch
from
bb31f47
to
b7e28d4
Compare
… david/delay-beefy-on-major-sync
… david/delay-beefy-on-major-sync
acatangiu
added
A0-please_review
client/beefy/src/worker.rs
Outdated
| @@ -400,6 +409,11 @@ where | |||
| )); | |||
|
|
|||
| loop { | |||
| if self.sync_oracle.is_major_syncing() { | |||
| debug!(target: "beefy", "Skipping initialization due to sync."); | |||
There was a problem hiding this comment.
This is not necessarily initialization, it is the main loop of the worker. I would rephrase to smth like:
| debug!(target: "beefy", "Skipping initialization due to sync."); | |
| debug!(target: "beefy", "Waiting for major sync to complete."); |
client/beefy/src/lib.rs
Outdated
| N: GossipNetwork<B> + Clone + Send + 'static, | ||
| SO: SyncOracle + Send + Sync + Clone + 'static, |
There was a problem hiding this comment.
Would these two ever be provided by different objects? If not, I think it would make sense to simply squash them together, they're both mandatory anyway...
client/beefy/src/worker.rs
Outdated
| if sync_oracle.is_major_syncing() { | ||
| Poll::Pending | ||
| } else { | ||
| Poll::Ready(()) | ||
| } | ||
| }) |
There was a problem hiding this comment.
this function has no way of knowing when the sync is over, i suggest adding it to the sync_oracle itself
There was a problem hiding this comment.
What if it's rescheduled for polling with cx.waker().wake_by_ref() before returning pending, Would that be fine?
client/beefy/src/worker.rs
Outdated
| ) -> impl future::Future<Output = ()> { | ||
| return future::poll_fn(move |cx| { | ||
| if sync_oracle.is_major_syncing() { | ||
| cx.waker().wake_by_ref(); |
There was a problem hiding this comment.
this is equivalent to a busy-wait loop, right? Like @seunlanlege said, this future needs to be tied to the sync_oracle waker.
… david/delay-beefy-on-major-sync
… david/delay-beefy-on-major-sync
…7/substrate into david/delay-beefy-on-major-sync
acatangiu
removed
B0-silent
|
Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions. |
stale
bot
added
the
A5-stale
|
Bump |
stale
bot
removed
the
A5-stale
|
Alternative PR #10882 merged |
Fixes paritytech/grandpa-bridge-gadget#112