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

Check context before notifying frontend and scheduler #5565

Merged
merged 3 commits into from
Sep 26, 2023

Conversation

justinjung04
Copy link
Contributor

@justinjung04 justinjung04 commented Sep 14, 2023

What this PR does:

We have two extra logs when the query times out:

ts=2023-08-29T15:57:55.2976267Z caller=scheduler_processor.go:187 level=info msg="finished request" status_code=499 response_size=95

ts=2023-08-29T15:57:55.297738564Z caller=scheduler_processor.go:211 level=error msg="error notifying frontend about finished query" err="rpc error: code = Canceled desc = context canceled" frontend=10.1.121.156:9095

ts=2023-08-29T15:57:55.297766783Z caller=scheduler_processor.go:163 level=error msg="error notifying scheduler about finished query" err=EOF addr=10.1.126.155:9095

We can check the context before notifying the frontend and scheduler if context error is not nil.

Which issue(s) this PR fixes:
n/a

Checklist

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

Signed-off-by: Justin Jung <jungjust@amazon.com>
Signed-off-by: Justin Jung <jungjust@amazon.com>
@harry671003
Copy link
Contributor

Could you help me understand why the extra logs are a problem?

@justinjung04
Copy link
Contributor Author

They're not really problematic, just want to remove extra error logs that's not helpful

@justinjung04 justinjung04 marked this pull request as ready for review September 15, 2023 15:40
Signed-off-by: Justin Jung <jungjust@amazon.com>
@yeya24 yeya24 merged commit cbcf039 into cortexproject:master Sep 26, 2023
14 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.

3 participants