Skip to content

Conversation

jxs
Copy link
Member

@jxs jxs commented Oct 3, 2025

Description

This is the up-streaming of sigp#570 which has been beeing used by https://github.com/sigp/lighthouse/ for some weeks now:

This started with an attempt to solve #5751 using the previous internal async-channel.
After multiple ideas were discussed off band, replacing the async-channel with an internal more tailored priority queue seemed inevitable. This priority queue allows us to implement the cancellation of in flight IDONTWANT's very cleanly with the remove_data_messages function. Clearing the stale messages likewise becomes simpler as we also make use of remove_data_messages .

Notes & open questions

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

jxs added 2 commits October 3, 2025 15:36
This started with an attempt to solve #5751 using the previous internal async-channel.
After multiple ideas were discussed off band, replacing the async-channel with an internal more tailored priority queue seemed inevitable.
This priority queue allows us to implement the cancellation of in flight IDONTWANT's very cleanly with the remove_data_messages function.
Clearing the stale messages likewise becomes simpler as we also make use of remove_data_messages
And this has the added advantage of being able to only have a single priority queue and making the code simpler.
If a peer is not making progress we can assume it's not delivering High priority messages and we can penalize it.
Copy link
Contributor

@AgeManning AgeManning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jxs jxs added send-it trivial Marks PRs which are considered trivial and don't need approval from another maintainer. and removed trivial Marks PRs which are considered trivial and don't need approval from another maintainer. labels Oct 7, 2025
@mergify mergify bot added the queued label Oct 7, 2025
@mergify mergify bot merged commit 2fb2486 into master Oct 7, 2025
69 of 71 checks passed
@mergify mergify bot deleted the gs-new-queue branch October 7, 2025 10:54
@mergify mergify bot removed the queued label Oct 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants