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

fix(compute/metadata): init a HTTP client in OnGCE check #5439

Merged
merged 2 commits into from Feb 3, 2022

Conversation

codyoss
Copy link
Member

@codyoss codyoss commented Feb 3, 2022

Initialize a resolver and http.Client locally in function to force
GC to cleanup background resources such as cached connections. This
can falsely report as leaked goroutines otherwise. This kind of issue
will still persist for calling methods like ProjectID, but we are
intentionally caching a client and its connections for these occasions
today. If this ends up causing users issues in the future we can
reevaluate at that time.

Fixes: #5430

Initalize a resolver and http.Client locally in function to force
GC to cleanup background resources such as cached connections. This
can falsely report as leaked goroutines otherwise. This kind of issue
will still persist for calling methods like ProjectID, but we are
intentionally caching a client and its connections for these occations
today. If this ends up causing users issues in the fututure we can
reevaluate at that time.

Fixes: googleapis#5430
@codyoss codyoss requested a review from a team as a code owner February 3, 2022 16:32
@product-auto-label product-auto-label bot added size: xs Pull request size is extra small. api: compute Issues related to the Compute Engine API. labels Feb 3, 2022
@codyoss
Copy link
Member Author

codyoss commented Feb 3, 2022

Note: this is a hunch kinda fix, but I have ran into errors like this in the past and I am pretty sure this was the solution.

Copy link
Contributor

@noahdietz noahdietz left a comment

Choose a reason for hiding this comment

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

After reading the issue, this fix makes sense to me. Initializing a client in the scope of the function should result in the client being tagged for GC clean up once the function is finished.

@codyoss codyoss enabled auto-merge (squash) February 3, 2022 16:42
@codyoss codyoss merged commit 76c6e40 into googleapis:main Feb 3, 2022
@release-please release-please bot mentioned this pull request Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: compute Issues related to the Compute Engine API. size: xs Pull request size is extra small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

compute/metadata: metadata.OnGCE() leaks 2 http goroutines
2 participants