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

objstore/azure: Only create an http client once #4928

Merged
merged 1 commit into from
Dec 7, 2021
Merged

objstore/azure: Only create an http client once #4928

merged 1 commit into from
Dec 7, 2021

Conversation

siggy
Copy link
Contributor

@siggy siggy commented Dec 7, 2021

getContainerURL was creating a new http.Client within HTTPSender.
This increased memory pressure on components making requests to Azure.

Move http.Client creation out of HTTPSender, only creating it once
per call to getContainerURL.

Signed-off-by: Andrew Seigner andrew@sig.gy

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

In objstore/azure, move http.Client creation out of HTTPSender, only creating it once
per call to getContainerURL.

Verification

We observed a 2x~4x memory increase in our alertmanager, compactor, ruler during an upgrade from Cortex v1.10.0 to v1.11.0, and tracked it down to a change in getContainerURL.

Memory usage of alertmanager, compactor, ruler with Cortex v1.10.0, v1.11.0, and this PR:

thanos mem fix

`getContainerURL` was creating a new `http.Client` within `HTTPSender`.
This increased memory pressure on components making requests to Azure.

Move `http.Client` creation out of `HTTPSender`, only creating it once
per call to `getContainerURL`.

Signed-off-by: Andrew Seigner <andrew@sig.gy>
@siggy siggy marked this pull request as ready for review December 7, 2021 19:25
@GiedriusS GiedriusS enabled auto-merge (squash) December 7, 2021 19:30
@GiedriusS GiedriusS merged commit c9d7465 into thanos-io:main Dec 7, 2021
@siggy siggy deleted the one-azure-client branch December 7, 2021 20:02
squat pushed a commit to squat/thanos that referenced this pull request Dec 8, 2021
`getContainerURL` was creating a new `http.Client` within `HTTPSender`.
This increased memory pressure on components making requests to Azure.

Move `http.Client` creation out of `HTTPSender`, only creating it once
per call to `getContainerURL`.

Signed-off-by: Andrew Seigner <andrew@sig.gy>
squat pushed a commit to squat/thanos that referenced this pull request Dec 8, 2021
`getContainerURL` was creating a new `http.Client` within `HTTPSender`.
This increased memory pressure on components making requests to Azure.

Move `http.Client` creation out of `HTTPSender`, only creating it once
per call to `getContainerURL`.

Signed-off-by: Andrew Seigner <andrew@sig.gy>
@squat squat mentioned this pull request Dec 8, 2021
squat pushed a commit that referenced this pull request Dec 9, 2021
`getContainerURL` was creating a new `http.Client` within `HTTPSender`.
This increased memory pressure on components making requests to Azure.

Move `http.Client` creation out of `HTTPSender`, only creating it once
per call to `getContainerURL`.

Signed-off-by: Andrew Seigner <andrew@sig.gy>
jordy1024 pushed a commit to jordy1024/thanos that referenced this pull request Dec 12, 2021
`getContainerURL` was creating a new `http.Client` within `HTTPSender`.
This increased memory pressure on components making requests to Azure.

Move `http.Client` creation out of `HTTPSender`, only creating it once
per call to `getContainerURL`.

Signed-off-by: Andrew Seigner <andrew@sig.gy>
jordy1024 pushed a commit to jordy1024/thanos that referenced this pull request Dec 12, 2021
`getContainerURL` was creating a new `http.Client` within `HTTPSender`.
This increased memory pressure on components making requests to Azure.

Move `http.Client` creation out of `HTTPSender`, only creating it once
per call to `getContainerURL`.

Signed-off-by: Andrew Seigner <andrew@sig.gy>
Signed-off-by: wenmaoba <wenmaoba@tencent.com>
pracucci pushed a commit to cortexproject/cortex that referenced this pull request Dec 14, 2021
* Update Thanos to latest main

Update Thanos dependency to include thanos-io/thanos#4928, to conserve
memory.

Signed-off-by: Andrew Seigner <andrew@sig.gy>

* Update changelog to summarize user-facing changes

Signed-off-by: Andrew Seigner <andrew@sig.gy>
alvinlin123 pushed a commit to ac1214/cortex that referenced this pull request Jan 14, 2022
* Update Thanos to latest main

Update Thanos dependency to include thanos-io/thanos#4928, to conserve
memory.

Signed-off-by: Andrew Seigner <andrew@sig.gy>

* Update changelog to summarize user-facing changes

Signed-off-by: Andrew Seigner <andrew@sig.gy>
Signed-off-by: Alvin Lin <alvinlin@amazon.com>
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.

2 participants