fix: resolve query lock bottleneck, relay finder untracked worker, and metrics pool memory leak - #3522
Open
Sahil-4555 wants to merge 9 commits into
Open
fix: resolve query lock bottleneck, relay finder untracked worker, and metrics pool memory leak#3522Sahil-4555 wants to merge 9 commits into
Sahil-4555 wants to merge 9 commits into
Conversation
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
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.
I saw some concurrency, lifecycle and memory management issues in the codebase that can lead to performance bottlenecks, untracked background goroutines and memory retention. I fixed these three problems in this PR.
Here are the issues and what we have done to address them:
1. DHT query events mutex lock contention
eventChannel.send, the mutexe.muwas held while executing a blockingselectstatement waiting to send an event into the channel. If the consumer was slow (the channel buffer was full) the write goroutine would block in the select, still holding the mutex. This caused all other concurrent writers/publishers to block immediately one.mu.Lock(), fully serialising the event publishing path.selectblock. To cover the case wherewaitThenClosecloses the channel concurrently, we added a deferredrecover()block that catches and safely ignores the closed-channel panic.2. Found Untracked Goroutine in Relay Finder
go rf.cleanupDisconnectedPeers(ctx)was launched on startup and not registered withrf.refCount(WaitGroup). Consequently, callingStop()would return before this cleanup loop had fully exited, which could lead to subscription leaks or test flakiness during host shutdown.rf.refCount.Add(1)and deferredDone()the right way, wrapping call inrf.refCounttracking.3. Metrics Helper Reference Preservation of
sync.Poolmetricshelperwere reset with(*s)[:0]. The slice to 0 resets the length, but the elements of the backing array (the string headers) are not touched. This prevents GC to free the referenced strings and the backing byte arrays, resulting in a hidden memory leak/retention.clear(*s)builtin insidePutStringSlicebefore returning the slice pointer to the pool so GC can clean up the memory immediately.