Skip to content
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

Add querier.max-subquery-steps to make subquery step size check optional #5656

Merged
merged 4 commits into from
Nov 16, 2023

Conversation

yeya24
Copy link
Contributor

@yeya24 yeya24 commented Nov 15, 2023

What this PR does:

Expose querier.max-subquery-steps subquery max allowed steps limit as a flag. This default limit is 0, which disables this check so that we can keep the behavior the same as previous releases and Prometheus

Which issue(s) this PR fixes:
Fixes #5651

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: Ben Ye <benye@amazon.com>
Signed-off-by: Ben Ye <benye@amazon.com>
Signed-off-by: Ben Ye <benye@amazon.com>
@yeya24 yeya24 requested a review from alanprot November 15, 2023 23:00
@alanprot
Copy link
Member

alanprot commented Nov 16, 2023

Thanks.. make sense to me!

But maybe in the future we should add a default as we know this can cause problems?

@yeya24
Copy link
Contributor Author

yeya24 commented Nov 16, 2023

But maybe in the future we should add a default as we know this can cause problems?

I see. I want to see if we can propose some kind of limit in Prometheus and keep Cortex behavior the same as Prometheus on this limit... Ideally this issue should be caught by samples limit, but there might be edge cases depending on actual query

@yeya24
Copy link
Contributor Author

yeya24 commented Nov 16, 2023

I will merge this one and keep this feature disabled in this release. We can think about whether we want to turn it on by defualt in the future

@yeya24 yeya24 merged commit 5a7228e into cortexproject:master Nov 16, 2023
14 checks passed
@yeya24 yeya24 deleted the disable-subquery-step-size-check branch November 16, 2023 02:40
yeya24 added a commit to yeya24/cortex that referenced this pull request Nov 16, 2023
…nal (cortexproject#5656)

* add querier.max-subquery-steps to make subquery step size check optional

Signed-off-by: Ben Ye <benye@amazon.com>

* update

Signed-off-by: Ben Ye <benye@amazon.com>

* disable subquery step size check by default, make it optional

Signed-off-by: Ben Ye <benye@amazon.com>

* fix integ test and add changelog

Signed-off-by: Ben Ye <benye@amazon.com>

---------

Signed-off-by: Ben Ye <benye@amazon.com>
yeya24 added a commit that referenced this pull request Nov 16, 2023
* Fix param_query omitted in query frontend query stats log (#5655)

* fix param_query not logged in query stats log

Signed-off-by: Ben Ye <benye@amazon.com>

* fix lint

Signed-off-by: Ben Ye <benye@amazon.com>

* fix unit test of user agent

Signed-off-by: Ben Ye <benye@amazon.com>

---------

Signed-off-by: Ben Ye <benye@amazon.com>

* Add querier.max-subquery-steps to make subquery step size check optional (#5656)

* add querier.max-subquery-steps to make subquery step size check optional

Signed-off-by: Ben Ye <benye@amazon.com>

* update

Signed-off-by: Ben Ye <benye@amazon.com>

* disable subquery step size check by default, make it optional

Signed-off-by: Ben Ye <benye@amazon.com>

* fix integ test and add changelog

Signed-off-by: Ben Ye <benye@amazon.com>

---------

Signed-off-by: Ben Ye <benye@amazon.com>

* bump RC version to 1.16.0-rc.1

Signed-off-by: Ben Ye <benye@amazon.com>

---------

Signed-off-by: Ben Ye <benye@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable subquery step size limit by default
2 participants