Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve tracing #873

Merged
merged 3 commits into from
Sep 4, 2023
Merged

Improve tracing #873

merged 3 commits into from
Sep 4, 2023

Conversation

Jorropo
Copy link
Contributor

@Jorropo Jorropo commented Aug 23, 2023

This reuse libp2p/go-libp2p-routing-helpers#80 which logs all inputs and outputs

@Jorropo Jorropo requested review from guillaumemichel and a team as code owners August 23, 2023 22:50
query.go Outdated Show resolved Hide resolved
@dennis-tra
Copy link
Contributor

If the channel was unbuffered that would leak go routines. However, the channel is buffered with alpha slots and we only spawn alpha queries.

query.go Outdated Show resolved Hide resolved
@Jorropo Jorropo requested a review from a team as a code owner August 25, 2023 05:20
@Jorropo Jorropo changed the title fix: deadlock in query Improve tracing Aug 25, 2023
@Jorropo
Copy link
Contributor Author

Jorropo commented Aug 25, 2023

If the channel was unbuffered that would leak go routines. However, the channel is buffered with alpha slots and we only spawn alpha queries.

Indeed I just got lucky and tests were passing at a moment, the actual issue was that Kubo were registering routing events and then synchronously call findprovs, however findprovs might not return until we first find something in the parallel routing helpers, so kubo was not draining the routing events and thus it was blocking the dht's logic.
I moved the call to FindProvidersAsync in the goroutine Kubo were already using and now it's good.

@guillaumemichel
Copy link
Contributor

LGTM, but let's wait for libp2p/go-libp2p-routing-helpers#80 to land before we merge this PR.

MB Copy Paste Typo,

This caused panics while tracing.
go-libp2p-routing-helpers has an optimized implementation that does nothing if we are not tracing, it also properly log all IO of the request.
We always select it against ctx and we don't rely on the fact it will always be non blocking since when count is zero we can't realistically preallocate.
Buffering use more memory makes more garbage and is less efficient than direct copies.
@Jorropo Jorropo merged commit 68c5dee into master Sep 4, 2023
9 checks passed
@Jorropo Jorropo deleted the fix-deadlock branch September 4, 2023 14:31
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