-
Notifications
You must be signed in to change notification settings - Fork 810
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 new query inflight request on ingester #6081
Add new query inflight request on ingester #6081
Conversation
Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com>
f120c4e
to
509dbab
Compare
Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com>
0ea7514
to
45c104e
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.
Interesting. Thanks! LGTM
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.
Thanks for doing this. I was wondering why defer is used in some places and not others.
@CharlieTLe |
Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com>
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.
Thanks
What this PR does:
This PR extended work done at #5798.
From tests, it was noticed that the metrics is accurate but does not show the impacting volume on ingester. I am proposing to remove from the metric any pre and post execution and count the metric only when the ingester is actually resolving the query. That from tests is when we see issues with CPU usage on ingesters.
Also adding a new ingester instance limit, similar to the existent
max_inflight_push_requests
to limit the max number of inflight queries on ingesters. The reads are from any GET operation on ingesters.Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]