PeerManager: add separate queue for gossip broadcasts#1683
Merged
TheBlueMatt merged 3 commits intolightningdevkit:mainfrom Aug 25, 2022
Merged
PeerManager: add separate queue for gossip broadcasts#1683TheBlueMatt merged 3 commits intolightningdevkit:mainfrom
TheBlueMatt merged 3 commits intolightningdevkit:mainfrom
Conversation
5f4e243 to
758561d
Compare
dunxen
reviewed
Aug 25, 2022
758561d to
91b113d
Compare
TheBlueMatt
reviewed
Aug 25, 2022
Collaborator
TheBlueMatt
left a comment
There was a problem hiding this comment.
LGTM. Can we now drop the enqueue_encoded_message utility now and only use the gossip variant?
91b113d to
50a417d
Compare
50a417d to
0fce80d
Compare
Codecov Report
@@ Coverage Diff @@
## main #1683 +/- ##
==========================================
- Coverage 90.91% 90.85% -0.06%
==========================================
Files 85 85
Lines 45753 45763 +10
Branches 45753 45763 +10
==========================================
- Hits 41596 41580 -16
- Misses 4157 4183 +26
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
jkczyz
reviewed
Aug 25, 2022
Contributor
jkczyz
left a comment
There was a problem hiding this comment.
Looks good but should make commits atomic as per comment.
0fce80d to
9b3fe9e
Compare
Collaborator
|
If you're referring to another PR in the git commit, please refer to the specific commit by commit hash, not to github - git history should stand on its own without context from github. |
Fixes a flipped bool that was introduced in 4a1ee5f
It's more accurate to name it as dropping gossip broadcasts, as it won't drop all gossip.
This allows us to better prioritize channel messages over gossip broadcasts and lays groundwork for rate limiting onion messages more simply, since they won't be competing with gossip broadcasts for space in the main message queue.
9b3fe9e to
47e818f
Compare
jkczyz
approved these changes
Aug 25, 2022
TheBlueMatt
approved these changes
Aug 25, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This allows us to better prioritize channel messages over gossip broadcasts and lays groundwork for rate limiting onion messages more simply, since they won't be competing with gossip broadcasts in the channel message queue.
Also fixes a bug that was introduced in #1660, oops.
Lays groundwork for #1604