apollo_propeller: wake poll from on_behaviour_event

[sirandreww-starkware]

Note

Low Risk
Small, localized change to async wakeup behavior in the connection handler; primary risk is unintended extra wakeups or missed wakeups if waker handling is incorrect.

Overview
The Propeller ConnectionHandler now caches the latest task Waker from poll and explicitly wakes it when new messages are enqueued via on_behaviour_event.

It also wakes the handler after DialUpgradeError handling (after resetting outbound state / clearing the send queue) so the connection can promptly progress without waiting for an external poll.

^{Written by Cursor Bugbot for commit f340c33. This will update automatically on new commits. Configure here.}

[reviewable-StarkWare]

This change is 

[sirandreww-starkware]

• apollo_propeller: handler fixes #13291
• apollo_propeller: handler tests #13290
• apollo_propeller: rate-limit invalid unit warnings in handler #13583
• apollo_propeller: remove HandlerOut::Unit from behaviour event path #13351
• apollo_propeller: guard inbound reads on non-empty unsent_units buffer #13582
• apollo_propeller: route inbound units through bounded channel #13350
• apollo_propeller: buffer unsent units from partial batches #13349
• apollo_propeller: wire bounded unit channels between handler and engine #13348
• apollo_propeller: add RegisterHandler command and inbound unit SelectAll #13347
• apollo_propeller: add inbound_channel_capacity config field #13346
• apollo_propeller: add backpressure flood test #13345
• apollo_propeller: wake poll from on_behaviour_event #13334 👈 (View in Graphite)
• apollo_propeller: drain send queue on DialUpgradeError #13332
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[guy-starkware]

@guy-starkware reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ShahakShama and sirandreww-starkware).

---

crates/apollo_propeller/src/handler.rs line 507 at r1 (raw file):

        // Store the waker so on_behaviour_event / on_connection_event can wake us.
        // Only clone when we're actually going to sleep (Pending), and skip if unchanged.
        if result.is_pending() && !self.waker.as_ref().is_some_and(|w| w.will_wake(cx.waker())) {

I'm not going to block you on this, and honestly I don't know what you can do... but this conditional (starting with the !) is not super readable. Don't spend too much time on it though...

[ShahakShama]

@ShahakShama reviewed all commit messages and made 1 comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on sirandreww-starkware).

---

crates/apollo_propeller/src/handler.rs line 507 at r1 (raw file):

        // Store the waker so on_behaviour_event / on_connection_event can wake us.
        // Only clone when we're actually going to sleep (Pending), and skip if unchanged.
        if result.is_pending() && !self.waker.as_ref().is_some_and(|w| w.will_wake(cx.waker())) {

What's the harm if we always store the waker? Say both to me and to the comment

[cursor]

PR Summary

Medium Risk
Touches async wakeup/polling behavior in a libp2p ConnectionHandler; incorrect waker usage could cause missed wakeups or extra churn, though the change is small and localized.

Overview
Improves Propeller connection-handler liveness by storing the most recent task Waker from ConnectionHandler::poll and using it to wake the handler when on_behaviour_event enqueues a new SendUnit.

Also wakes the handler after DialUpgradeError handling resets outbound state/clears the send queue, so the handler promptly re-polls to emit queued error events or re-establish an outbound substream.

^{Reviewed by Cursor Bugbot for commit 8b47ad3. Bugbot is set up for automated code reviews on this repo. Configure here.}

[sirandreww-starkware]

@sirandreww-starkware made 2 comments.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on guy-starkware and ShahakShama).

---

crates/apollo_propeller/src/handler.rs line 507 at r1 (raw file):

Previously, guy-starkware wrote…

I'm not going to block you on this, and honestly I don't know what you can do... but this conditional (starting with the !) is not super readable. Don't spend too much time on it though...

Simplified — now always store the waker, dropping the conditional entirely. Wakers are cheap Arc clones, so the optimization wasn't worth the readability cost.

---

crates/apollo_propeller/src/handler.rs line 507 at r1 (raw file):

Previously, ShahakShama wrote…

What's the harm if we always store the waker? Say both to me and to the comment

No harm — Waker is Arc-backed so clone() is just a refcount bump. Switched to unconditionally storing the waker and trimmed the comment to match.

[ShahakShama]

@ShahakShama reviewed 1 file and all commit messages, made 2 comments, and resolved 1 discussion.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on guy-starkware, ShahakShama, and sirandreww-starkware).

---

crates/apollo_propeller/src/handler.rs line 76 at r2 (raw file):

    /// Maximum wire message size for batching.
    max_wire_message_size: usize,
    /// The most recent waker from [`ConnectionHandler::poll`], used to wake the task when new

used to sounds like it used to but it isn't now
I think used for waking is the best but maybe there are other better options

[graphite-app]

Merge activity

• Apr 30, 8:58 AM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

[sirandreww-starkware]

@sirandreww-starkware reviewed 1 file and all commit messages, and resolved 2 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on sirandreww-starkware).