Revoke storing target and metadata cache in context. #10590
Merged
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
Storing the scrape cache and the target (which also contains that cache) is apparently causing hige memory increase. I think me might not control the lifespan of the context enough, therefore old objects keep living in memory for longer than needed. Let's unblock the release and look for an alternative so that downstream consumers can get access to that data. Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>
|
/prombench v2.34.0 |
|
maybe #10588 fixes this too |
3 tasks
|
/prombench cancel |
|
Benchmark cancel is in progress. |
|
Hi, sorry for all the trouble. I'll look into this now, but just to be clear, #10588 did not fix it right? |
No, it did not. we immediately see huge increase in RSS in prombench. |
|
thanks @roidelapluie! double cross finger it fixed the memory leak |
gouthamve
added a commit
that referenced
this pull request
Apr 27, 2022
The previous attempt was rolled back in #10590 due to memory issues. `sl.parentCtx` and `sl.ctx` both had a copy of the cache and target info in the previous attempt and it was hard to pin-point where the context was being retained causing the memory increase. I've experimented a bunch in #10627 to figure out that this approach doesn't cause memory increase. Beyond that, just using this info in _any_ other context is causing a memory increase. The change fixed a bunch of long-standing in the OTel Collector that the community was waiting on and release is blocked on a few downstream distrubutions of OTel Collector waiting on a fix. I propose to merge this change in while I investigate what is happening. Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
gouthamve
added a commit
that referenced
this pull request
May 2, 2022
The previous attempt was rolled back in #10590 due to memory issues. `sl.parentCtx` and `sl.ctx` both had a copy of the cache and target info in the previous attempt and it was hard to pin-point where the context was being retained causing the memory increase. I've experimented a bunch in #10627 to figure out that this approach doesn't cause memory increase. Beyond that, just using this info in _any_ other context is causing a memory increase. The change fixed a bunch of long-standing in the OTel Collector that the community was waiting on and release is blocked on a few downstream distrubutions of OTel Collector waiting on a fix. I propose to merge this change in while I investigate what is happening. Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
gouthamve
added a commit
that referenced
this pull request
May 3, 2022
* Send target and metadata cache in context (again) The previous attempt was rolled back in #10590 due to memory issues. `sl.parentCtx` and `sl.ctx` both had a copy of the cache and target info in the previous attempt and it was hard to pin-point where the context was being retained causing the memory increase. I've experimented a bunch in #10627 to figure out that this approach doesn't cause memory increase. Beyond that, just using this info in _any_ other context is causing a memory increase. The change fixed a bunch of long-standing in the OTel Collector that the community was waiting on and release is blocked on a few downstream distrubutions of OTel Collector waiting on a fix. I propose to merge this change in while I investigate what is happening. Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com> * Gate the change behind a manager option Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
roidelapluie
added a commit
to roidelapluie/prometheus
that referenced
this pull request
Jun 22, 2022
Storing the scrape cache and the target (which also contains that cache) is apparently causing hige memory increase. I think me might not control the lifespan of the context enough, therefore old objects keep living in memory for longer than needed. Let's unblock the release and look for an alternative so that downstream consumers can get access to that data. Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>
roidelapluie
pushed a commit
to roidelapluie/prometheus
that referenced
this pull request
Jun 22, 2022
* Send target and metadata cache in context (again) The previous attempt was rolled back in prometheus#10590 due to memory issues. `sl.parentCtx` and `sl.ctx` both had a copy of the cache and target info in the previous attempt and it was hard to pin-point where the context was being retained causing the memory increase. I've experimented a bunch in prometheus#10627 to figure out that this approach doesn't cause memory increase. Beyond that, just using this info in _any_ other context is causing a memory increase. The change fixed a bunch of long-standing in the OTel Collector that the community was waiting on and release is blocked on a few downstream distrubutions of OTel Collector waiting on a fix. I propose to merge this change in while I investigate what is happening. Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com> * Gate the change behind a manager option Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
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.
Storing the scrape cache and the target (which also contains that cache)
is apparently causing hige memory increase. I think me might not control
the lifespan of the context enough, therefore old objects keep living in
memory for longer than needed.
Let's unblock the release and look for an alternative so that downstream
consumers can get access to that data.
Signed-off-by: Julien Pivotto roidelapluie@o11y.eu