Only consider an interval to have possible expired chunks if it overlaps a delete. #6297
Conversation
|
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0.7% |
| d.deleteRequestsToProcessMtx.Lock() | ||
| defer d.deleteRequestsToProcessMtx.Unlock() | ||
|
|
||
| if userID != "" { | ||
| for _, deleteRequest := range d.deleteRequestsToProcess { | ||
| if deleteRequest.UserID == userID { | ||
| if deleteRequest.UserID == userID && | ||
| deleteRequest.StartTime <= interval.End && |
There was a problem hiding this comment.
In your comment on the PR you mention if an interval overlaps a delete request, but this logic requires the interval to be fully within the delete request time period (so excluding an overlap at the start or end of the interval). Is that intended? Would there be a situation where we'd want an || here instead?
There was a problem hiding this comment.
This actually handles overlaps because any overlapping interval will always start before the end of the delete and end after the start of the delete. Fully overlapping is just a special case. I've included a picture
Before, we considered any interval to potentially have an expired chunk if the passed userID matched a user on a DeleteRequest. This PR adds the requirement that an interval must also overlap a delete request to count as having a potential expired chunk.