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

Configurable maximum of the limits parameter. #1795

Closed
cyriltovena opened this issue Mar 11, 2020 · 4 comments · Fixed by #1798
Closed

Configurable maximum of the limits parameter. #1795

cyriltovena opened this issue Mar 11, 2020 · 4 comments · Fixed by #1798
Labels
component/loki help wanted We would love help on these issues. Please come help us! type/enhancement Something existing could be improved

Comments

@cyriltovena
Copy link
Contributor

cyriltovena commented Mar 11, 2020

Describe the bug

Since sending a huge limits request can crash Loki. We should have a configurable maximum of the limit the users can requests. This should be configurable per tenant.

To Reproduce

100k line requests below crashes Loki.

time curl 'https://localhost:3100/loki/api/v1/query_range?direction=FORWARD&end=1583866800000000000&limit=100000&query=%7Bcluster%3D%22us-central1%22%2C+namespace%3D%22prod%22%2C+job%3D%22prod%2Fquerier%22%7D+%7C%3D+%22GET+%2Fapi%2Fprom%2Fapi%2Fv1%2Fquery_range%22&start=1583863200000000000' -v | jq .data.result[].values[][1] | wc -l

Expected behavior

We should not crash and be able to limit.

/cc @owen-d @slim-bean

@cyriltovena cyriltovena added component/loki help wanted We would love help on these issues. Please come help us! type/enhancement Something existing could be improved labels Mar 11, 2020
@adityacs
Copy link
Contributor

@cyriltovena Would like to work on this. However, need your guidance

@cyriltovena
Copy link
Contributor Author

Sure this is somehow similar to this commit d6b8587

We need a new overrides and config to limit to a maximum the limit query string.

Then you want to check in the querier and in the frontend this limit and return a 400. We don't need a metric for when someone is hitting this limit.

Pretty sure you can do this easily !

@adityacs
Copy link
Contributor

adityacs commented Mar 12, 2020

@cyriltovena Just thinking on this. May be the idea is silly.
Can we make this a dynamic parameter by looking into the memory available for loki(querier and frontend incase of microservice mode) to load the entries? If the user requested limit is more than the entries we can load with available memory, we can return an error.

@cyriltovena
Copy link
Contributor Author

It's not a silly idea but there is more into the play than just the limit which makes it too complicated. I more entitled to use a simple solution like the limit configuration that the operator can tweak on demand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/loki help wanted We would love help on these issues. Please come help us! type/enhancement Something existing could be improved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants