Store: Document --chunk-pool-size better

[daixiang0]

Signed-off-by: Xiang Dai [email protected]
Fix #2042

[squat]

Hmm this change makes the comment more confusing IMO

[daixiang0]

@squat how about Maximum size of concurrently allocatable bytes for chunks in memory, not cache.?

[GiedriusS]

@squat how about Maximum size of concurrently allocatable bytes for chunks in memory, not cache.?

Not sure about the , not cache. part because I can imagine there will be a question about what kind of cache are we exactly talking about but otherwise sounds good. It seems to me like the goal here is to put emphasis on the fact that we are talking about the memory in general. Let's see what @squat and others think.

[squat]

I agree with @GiedriusS 👍

[daixiang0]

I think it means some fixed memory during thanos running, not the cache memory.

[bwplotka]

That's the problem @daixiang0. It's not a fixed memory. It does not allocate that much as you put, it's only use a maximum of configured memory for reusable bytes pool. And if it will go beyond that for chunks it will fail the request. In theory. In practice, I really could not see our:

// ErrPoolExhausted is returned if a pool cannot provide the request bytes.
var ErrPoolExhausted = errors.New("pool exhausted")

But that's another story (:

[daixiang0]

@bwplotka does it mean that memory leak or the speed of system clean cache memory is faster than thanos consumes?

[bwplotka]

No, it means opposite: garbage collection (cleaning) is slower than our allocations, even though we don't need some memory. That's why we pool memory.

[bwplotka]

Thank you! LGTM 👍