Skip to content

Conversation

fjl
Copy link
Contributor

@fjl fjl commented Aug 29, 2025

This improves the latency of lookups in small networks and test setups. When the local node table runs empty, the lookupIterator will trigger refresh to try and fill the table again.

@fjl fjl marked this pull request as ready for review September 5, 2025 09:16
@fjl fjl requested a review from zsfelfoldi as a code owner September 5, 2025 09:16
fjl added 10 commits September 10, 2025 20:10
The lookup would add self into the replyBuffer if returned by another node.
Avoid doing that by marking self as seen.

With the changed initialization behavior of lookup, the lookupIterator needs to yield the
buffer right after creation. This fixes the smallNetConvergence test, where all results
are straight out of the local table.
@fjl fjl force-pushed the p2p-discover-waitfornodes branch from 1955eb1 to a9f9e0d Compare September 10, 2025 18:10
@fjl fjl added this to the 1.16.4 milestone Sep 11, 2025
We have changed this behavior, better clarify in comment.

Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
fjl and others added 2 commits September 12, 2025 12:50
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>

# Conflicts:
#	p2p/discover/lookup.go
cskiraly
cskiraly previously approved these changes Sep 16, 2025
Copy link
Contributor

@cskiraly cskiraly left a comment

Choose a reason for hiding this comment

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

LGTM

using it.lookup.tab inside is unsafe

Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
@cskiraly cskiraly merged commit a4c9b34 into ethereum:master Sep 17, 2025
5 of 6 checks passed
fjl added a commit that referenced this pull request Oct 15, 2025
This fixes a regression introduced in #32518. In that PR, we removed the
slowdown logic that would throttle lookups when the table runs empty.
Said logic was originally added in #20389.

Usually it's fine, but there exist pathological cases, such as hive
tests, where the node can only discover one other node, so it can only
ever query that node and won't get any results. In cases like these, we
need to throttle the creation of lookups to avoid crazy CPU usage.
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.

3 participants