Skip to content

p2p/enode: use per-source timeout in FairMix.Next#18046

Merged
anacrolix merged 2 commits into
erigontech:mainfrom
Fibonacci747:fix/enode-fairmix-per-source-timeout
Dec 3, 2025
Merged

p2p/enode: use per-source timeout in FairMix.Next#18046
anacrolix merged 2 commits into
erigontech:mainfrom
Fibonacci747:fix/enode-fairmix-per-source-timeout

Conversation

@Fibonacci747
Copy link
Copy Markdown
Contributor

This change fixes an issue where mixSource.timeout was initialized and mutated but never actually used to drive the timer in FairMix.Next. Previously the timer was based on the global m.timeout, so the per-source exponential backoff logic had no effect and the timeout field was effectively dead code. The fix moves timer creation after picking the source and bases it on source.timeout, resetting it to m.timeout on successful reads and halving it on timeouts. This matches the upstream intent (see go-ethereum PR #25962 ethereum/go-ethereum#25962 ) and restores per-source backoff, improving responsiveness when individual sources stall, while preserving the negative-timeout “completely fair” behavior.

@anacrolix
Copy link
Copy Markdown
Contributor

@Fibonacci747 any reason you didn't take teh whole diff from upstream?

@Fibonacci747
Copy link
Copy Markdown
Contributor Author

@Fibonacci747 any reason you didn't take teh whole diff from upstream?

Now took whole diff

@anacrolix
Copy link
Copy Markdown
Contributor

@Fibonacci747 what about the comments 😓

Copy link
Copy Markdown
Contributor

@anacrolix anacrolix left a comment

Choose a reason for hiding this comment

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

Fine to merge this when the comments are included

@Fibonacci747
Copy link
Copy Markdown
Contributor Author

Fine to merge this when the comments are included

But comments are uncluded? you mean comments on line 222-223, 231-233? ethereum/go-ethereum@c0135a8

@anacrolix
Copy link
Copy Markdown
Contributor

I might have missed them in the follow up review, GitHub likes to jump the gun and show diffs of what changed rather than the full PR. Thanks for the contribution

@anacrolix anacrolix merged commit e9f961f into erigontech:main Dec 3, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants