fix: prevent the goroutine leak from lb health checks #10
Conversation
This file contains 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
We had a leak of goroutines where the `lb.Start()` failed due to a port conflict. The health check goroutines were never terminated, were waiting on the context. This caused a leak of goroutines over a long time (as every failure on `.Start()` caused an exponential backoff) and increased CPU usage. Fix this by calling `lb.Close()` even if starting the load balancer has failed. Additionally, add the following features: - allow running an optional pprof server - allow specifying disallowed host port ranges to not be attempted to be proxied Also rekres & bump deps. Signed-off-by: Utku Ozdemir <[email protected]>
utkuozdemir
commented
Aug 14, 2024
Comment on lines
+135
to
+137
| // we still need to close the loadbalancer, so that the health checks goroutines get terminated | ||
| if closeErr := lb.Close(); closeErr != nil { | ||
| return errors.Join(fmt.Errorf("failed to start loadbalancer: %w", err), fmt.Errorf("failed to close loadbalancer: %w", closeErr)) |
There was a problem hiding this comment.
this is the actual fix of the goroutine leak
frezbo
approved these changes
Aug 14, 2024
smira
approved these changes
Aug 14, 2024
|
/m |
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.
We had a leak of goroutines where the
lb.Start()failed due to a port conflict. The health check goroutines were never terminated, were waiting on the context.This caused a leak of goroutines over a long time (as every failure on
.Start()caused an exponential backoff) and increased CPU usage.Fix this by calling
lb.Close()even if starting the load balancer has failed.Additionally, add the following features:
Also rekres & bump deps.