TSDB Index reuses slices, adds pools #5630
Conversation
owen-d
changed the title
pkg/storage/tsdb/multi_file_index.go
Outdated
| groups, err := i.forIndices(ctx, from, through, func(ctx context.Context, idx Index) (interface{}, error) { | ||
| return idx.GetChunkRefs(ctx, userID, from, through, shard, matchers...) | ||
| refs := ChunkRefsPool.Get() | ||
| err := idx.GetChunkRefs(ctx, userID, from, through, &refs, shard, matchers...) |
There was a problem hiding this comment.
if the final append was inside here you wouldn't need to Get from the pool multiple times, just reset the slice for each indice.
Same for the Multi Series.
There was a problem hiding this comment.
I used independent slices because i.forIndices runs all these calls in parallel.
pkg/storage/tsdb/multi_file_index.go
Outdated
| func (i *MultiIndex) GetChunkRefs(ctx context.Context, userID string, from, through model.Time, res *[]ChunkRef, shard *index.ShardAnnotation, matchers ...*labels.Matcher) error { | ||
| *res = (*res)[:0] |
There was a problem hiding this comment.
Where is (*MultiIndex).GetChunkRefs() called from? If a newly retrieved *[]ChunkRef is passed to the function, why do we need to reset the slice?
There was a problem hiding this comment.
This isn't actually called from anywhere yet -- we're just building the prototype for how we will interact with the tsdb-based index. Regarding *[]ChunkRef, it was to allow the caller to supply their own slice to avoid allocations. It's encouraged that the slice could be reused across multiple calls to avoid allocations, so we reset it.
This PR changes the index interface to accept slices. This allows us to reuse memory more easily. To make this easier, I've introduced a number of releveant pools for us to use.
It also adds a configurable
roundsparameter (default 8) for the benchmarking script.ref #5428