Skip to content

Fix TIME_WAIT connection leak from queriers to compactor #6681

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

Conversation

MasslessParticle
Copy link
Contributor

http://tleyden.github.io/blog/2016/11/21/tuning-the-go-http-client-library-for-load-testing/

This case is overwhelmingly likely to be from the connection pool as DefaultMaxIdleConnsPerHost is only 2. This PR also makes sure to drain the http response body, just in case.

@MasslessParticle MasslessParticle requested a review from a team as a code owner July 14, 2022 19:58
@MasslessParticle MasslessParticle added the backport release-2.6.x Tag a PR with this label to create a PR which cherry pics it into the release-2.6.x branch label Jul 14, 2022
Copy link
Contributor

@DylanGuedes DylanGuedes left a comment

Choose a reason for hiding this comment

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

lgtm. btw, first time seeing this ioutil.discard, interesting

@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
-               loki	-0.2%
@MasslessParticle MasslessParticle merged commit 25bd0a4 into grafana:main Jul 14, 2022
@grafanabot
Copy link
Collaborator

The backport to release-2.6.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-6681-to-release-2.6.x origin/release-2.6.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x 25bd0a4a55d84594df74a5c27e02ecab90130cd1
# Push it to GitHub
git push --set-upstream origin backport-6681-to-release-2.6.x
git switch main
# Remove the local backport branch
git branch -D backport-6681-to-release-2.6.x

Then, create a pull request where the base branch is release-2.6.x and the compare/head branch is backport-6681-to-release-2.6.x.

@dannykopping
Copy link
Contributor

Nice! I remember debugging this exact issue with Peter from Mimir last year on the billing forwarder:
https://github.com/grafana/backend-enterprise/pull/1472/files#diff-0e057ea5ec711488359552e50ba11ed62dceb0884abfa6c3c8654a82e7246080R174

PS the backporting seems to have failed?

vlad-diachenko pushed a commit that referenced this pull request Jul 15, 2022
* Fix TIME_WAIT connection leak from queriers to compactor

* lint
vlad-diachenko added a commit that referenced this pull request Jul 15, 2022
* Fix TIME_WAIT connection leak from queriers to compactor

* lint

Co-authored-by: Travis Patterson <travis.patterson@grafana.com>
sandeepsukhani pushed a commit that referenced this pull request Jul 21, 2022
* Fix TIME_WAIT connection leak from queriers to compactor

* lint
lxwzy pushed a commit to lxwzy/loki that referenced this pull request Nov 7, 2022
* Fix TIME_WAIT connection leak from queriers to compactor

* lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport release-2.6.x Tag a PR with this label to create a PR which cherry pics it into the release-2.6.x branch size/S
4 participants