fix: clear profile buffer at flush #3179
Conversation
| @@ -45,7 +41,7 @@ func NewMergeRowReader(readers []parquet.RowReader, maxValue parquet.Row, less f | |||
| } | |||
| its := make([]iter.Iterator[parquet.Row], len(readers)) | |||
| for i := range readers { | |||
| its[i] = NewBufferedRowReaderIterator(readers[i], defaultRowBufferSize) | |||
| its[i] = NewBufferedRowReaderIterator(readers[i], 1) | |||
There was a problem hiding this comment.
Experiments show that buffering here is not helpful; we reference many column readers but could release them earlier, allowing the GC to free unused objects. Rows in our case are too large (approximately 10KB) for batching.
We may want to refactor this piece: a buffer of size 1 does not significantly harm performance, but it also does not provide any benefits.
|
I noticed that the change causes notable increase of the working set size (the bottom boundary), which is not expected; debugging 
Update: I didn't find an explanation better than google/cadvisor#3286. I think the best we can do is to test it under memory pressure. |
kolesnikovae
force-pushed
the
fix/reclaim-profile-buffer-space
branch
from
38b9edc
to
25038be
Compare
|
Closing it for now: we do have some issues with how buffers are handled / retained, however, this fix does not solve them completely, and the benefit is barely measurable at scale. Instead, it makes sense to check how the row groups are merged at flush (we could probably avoid this altogether) |
When writing a chunk of profiles to disk, we allocate a buffer to hold the affected profiles, which we reuse throughout the lifetime of a mutable block in memory. Each entry in this buffer refers to a profile, including its metadata and samples.
The problem is that we do not clear this buffer after use, preventing the written profiles from being garbage collected, and thus consuming memory unnecessarily.
To compound the problem, creating a new parquet row group may require a considerable amount of memory, as evidenced by the spike in the graph. This makes the ingesters susceptible to OOM kills, even in relatively small deployments.
Notable changes proposed in the PR:
No noticeable increase in CPU consumption was identified. From the OS view, the working set size of the process will likely not change substantially, because of the FS cache.
The impact of the change is expected to be larger in large-scale deployments, where the amount of erroneously retained object and buffers is larger.