-
Notifications
You must be signed in to change notification settings - Fork 818
Chunk time ranges are inclusive. #1576
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Tom Wilkie <[email protected]>
Signed-off-by: Tom Wilkie <[email protected]>
Signed-off-by: Tom Wilkie <[email protected]>
Signed-off-by: Tom Wilkie <[email protected]>
1843e9e
to
b700996
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tested that tablemanager still works as expected? ISTR the "If through ends on 00:00 of the day, don't include the upcoming day" check is important there.
@@ -602,8 +602,8 @@ func TestChunkStoreRandom(t *testing.T) { | |||
|
|||
// pick two random numbers and do a query | |||
for i := 0; i < 100; i++ { | |||
start := rand.Int63n(100 * chunkLen) | |||
end := start + 1 + rand.Int63n((100*chunkLen)-start) | |||
start := rand.Int63n(99 * chunkLen) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we want this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the end of the list of chunks there are no more chunks. It simplifies the test logic below to ensuring we never query over the end of the list of chunks.
Not specifically, but we've been running this for ~3 weeks and not found any problems. |
Turns out I was thinking of a very similar line in LGTM |
Fixes #1522 "ChunkStoreRandom Flake".
I believe this was a real bug - queries that ended on exact day boundaries wouldn't query chunks which started on exact day boundaries.