-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Set receiver chunk trimming as a feature flag #7884
base: main
Are you sure you want to change the base?
Conversation
This commit disables chunk trimming by default and adds it as a feature flag. Signed-off-by: Filip Petkovski <[email protected]>
Signed-off-by: Filip Petkovski <[email protected]>
Signed-off-by: Filip Petkovski <[email protected]>
@@ -36,6 +36,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re | |||
- [#7855](https://github.com/thanos-io/thanos/pull/7855) Compcat/Query: Add support for comma separated replica labels. | |||
- [#7654](https://github.com/thanos-io/thanos/pull/7654) *: Add '--grpc-server-tls-min-version' flag to allow user to specify TLS version, otherwise default to TLS 1.3 | |||
- [#7854](https://github.com/thanos-io/thanos/pull/7854) Query Frontend: Add `--query-frontend.force-query-stats` flag to force collection of query statistics from upstream queriers. | |||
- [#7884](https://github.com/thanos-io/thanos/pull/7884) Receive: Add a `disable-chunk-trimming` feature which can be used to return entire chunks to queriers instead of cutting them to the requested range. |
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.
How would users know whether to enable this or not? Does this actually change the results?
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.
I wasn't able to run cortex integration tests locally using the official docs so I cannot reproduce the issue and verify why the test fails. @yeya24 do you happen to have the test case which reproduces this failure?
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.
#7815 (comment)
I only have the query here but I see it can fail for either start()
or end()
with timestamp
function.
Maybe we can mention the impacted query string in the changelog? If they don't run similar query they can enable the flag
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.
But once the chunks are in object storage, the result changes because Store doesn't trim them? So, it sounds like we always need to disable trimming.
This commit disables chunk trimming by default and adds it as a feature flag.
Changes
Verification