Compactor pooling changes #4985
Merged
Conversation
This file contains hidden or 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
…ool with lower fixed size
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.
What this PR does:
Compactor memory usage and frequency of OOMs has increased over time and investigation showed that almost all of it is via the parquet row pool. It's been hard to ascertain the exact reasons, but the pooling slices for large traces is not providing the benefit that it used to. It seems that the large amount of pooled data no longer gets GC'ed like expected and accumulates. I couldn't spot any bugs in repooling, so I think it is a change in behavior at runtime due to a combination of factors: vParquet4 event/link attribute columns significantly increasing row sizes vs the prior json blobs, golang runtime, and environmental factors.
This PR removes pooling for large traces, and it has been tested to have a huge improvement in memory, cutting memory high-water marks to less than half in a high traffic/tenant install.
Some amount of pooling is needed to keep cpu, throughput, and GC rate maintained. Now we have a fixed 1M default allocation size, which covers most traces.
Initial Finding
This profile prior to an OOM showed the row pool was holding 85% of inuse memory:
Graph
Benchmarks
Our synthetic benchmarks of course show the large increase in allocs, and don't do a good job of capturing the real-world performance. An area for improvement.
Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]