Fix rewrite chunk reuse reference #3710
Conversation
Signed-off-by: yeya24 <[email protected]>
| @@ -47,16 +47,17 @@ func (s *lazyPopulateChunkSeriesSet) Next() bool { | |||
| continue | |||
| } | |||
|
|
|||
| chks := make([]chunks.Meta, len(s.bufChks)) | |||
There was a problem hiding this comment.
Nice, so this is allowing us to not care about locking, but instead allocate more mem, right? 🤗
How benchmarks are looking with this?
There was a problem hiding this comment.
Sorry I didn't get the point. How is it related to locking? Did you mean adding a lock to protect s.bufChks?
I haven't benchmarked this. For a one-off job, memory seems not the highest priority. Maybe we can call GC manually somewhere if the mem usage is too high?
I am not sure if we can fix this without allocating additional memory. What do you think?
There was a problem hiding this comment.
I mean, no lock is needed because we copy the slice?
There was a problem hiding this comment.
Sorry but I think I missed something. Is s.bufChks accessed by multiple goroutines? I thought it is accessed sequentially.
There was a problem hiding this comment.
Let's restart this. Can you explain what problem are you solving? you mentioned in the description that you are fixing some problem, which one? (:
Just creating slices everywhere is not the best idea if we care about efficiency here, we never seen any error around this place on production too.
There was a problem hiding this comment.
I cannot tell tbh. But with this change the relabel pr works. I guess it is the problem of that pr because the rewrite delete works perfect.
But cannot figure out why.
There was a problem hiding this comment.
I cannot tell tbh. But with this change the relabel pr works. I guess it is the problem of that pr because the rewrite delete works perfect.
But cannot figure out why.
Signed-off-by: yeya24 [email protected]
Fix buf chunks reuse reference.
Here
mis a reference to the buffered chunk, we should use a copy instead.I can reproduce this problem while adding tests for #3707.
Changes
Verification