Bias Selector to first poll the sleeper future

[tnull]

Previously, lightning-background-processor's Selector would poll all other futures before finally polling the sleeper and returning the exit flag if it's ready. This could lead to scenarios where we infinitely keep processing background events and never respect the exit flag, as long as any of other futures keep being ready.

Here, we instead bias the Selector to always first poll the sleeper future, and hence have us act on the exit flag immediately if is set.

[ldk-reviews-bot]

👋 Thanks for assigning @TheBlueMatt as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

[codecov]

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.34%. Comparing base (de384ff) to head (9c0ca26).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning-background-processor/src/lib.rs	90.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4259   +/-   ##
=======================================
  Coverage   89.33%   89.34%           
=======================================
  Files         180      180           
  Lines      139042   139042           
  Branches   139042   139042           
=======================================
+ Hits       124219   124225    +6     
- Misses      12196    12200    +4     
+ Partials     2627     2617   -10     

Flag	Coverage Δ
fuzzing	35.00% <ø> (-0.97%)	⬇️
tests	88.70% <90.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

Did you experience this somewhere? Seems somewhat surprising that we'd have that much to process in the other futures.

[tnull]

Did you experience this somewhere? Seems somewhat surprising that we'd have that much to process in the other futures.

We got user reports regarding LDK Node timing out on trying to shutdown the background processor. While we still have to better understand why we would keep processing events, AFAICT the biased selector would likely have allowed us to exit the event processing loop nonetheless. Btw, we recently started doing the essentially same thing in LDK Node by using tokio::select's biased flage (cf. https://docs.rs/tokio/latest/tokio/macro.select.html#fairness) to ensure we process the stop event ASAP.

[TheBlueMatt]

Hmm, yea, seems like it might be worth digging into that. I don't think any of the futures being polled should complete instantly unless event handling is actually slower than events are coming in. I think the ldk-node sleeper correctly returns early if it wants to exit rather than waiting, so its also not that.

That said, I don't see a problem in biasing towards the sleeper, but please move it to A rather than biasing towards E, which just seems weird?

[tnull]

Hmm, yea, seems like it might be worth digging into that. I don't think any of the futures being polled should complete instantly unless event handling is actually slower than events are coming in. I think the ldk-node sleeper correctly returns early if it wants to exit rather than waiting, so its also not that.

Yup, unfortunately it's a bit unclear what happened as the event processing task just hung without any log output.

That said, I don't see a problem in biasing towards the sleeper, but please move it to A rather than biasing towards E, which just seems weird?

Of course, now pushed fixup that moves the sleeper to Selector::A.

[TheBlueMatt]

Feel free to squash.

[tnull]

Feel free to squash.

Squashed without further changes.

[ldk-reviews-bot]

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.