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 new query inflight request on ingester #6081

Merged
merged 3 commits into from
Jul 18, 2024

Conversation

danielblando
Copy link
Contributor

@danielblando danielblando commented Jul 11, 2024

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

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

Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com>
@danielblando danielblando marked this pull request as ready for review July 11, 2024 18:37
@CharlieTLe CharlieTLe changed the title Add new query infligh request on ingester Add new query inflight request on ingester Jul 11, 2024
Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com>
Copy link
Member

@friedrichg friedrichg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. Thanks! LGTM

Copy link
Member

@CharlieTLe CharlieTLe left a 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.

@danielblando
Copy link
Contributor Author

@CharlieTLe
I tried to add defer when possible. The places which have defer dont have any computation after it. It just check for error and then return. The places where we still do post execution I added the callback.
I did try to create a wrapper for those func. It started getting messy because each call is from a different interface and i thought was getting too complicated for a simple counter.

Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com>
Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@yeya24 yeya24 merged commit d2a0b02 into cortexproject:master Jul 18, 2024
16 checks passed
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.

5 participants