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 error code for queries killed due to too many tasks on cluster #24465

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Sullivan-Patrick
Copy link
Contributor

Description

In cases where a cluster needs to kill queries to stay under the configured threshold, we were using the same error code as cases where a query has more tasks than the configured threshold. This differentiates these and makes the former case an internal error.

Impact

In the first case, users will see updated error messaging reflecting the internal error.

Test Plan

Changes existing tests to reflect new error codes.

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.
== NO RELEASE NOTE ==

swapsmagic
swapsmagic previously approved these changes Jan 30, 2025
Copy link
Contributor

@swapsmagic swapsmagic left a comment

Choose a reason for hiding this comment

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

One minor comment, but rest looks good.

Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to remove this or it is safe to keep it?

Copy link
Contributor Author

@Sullivan-Patrick Sullivan-Patrick Jan 30, 2025

Choose a reason for hiding this comment

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

Sorry I don't follow- what are you referring to removing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well I don't understand what the changeset in GitHub is showing but it should be fixed :)

Copy link
Contributor

Choose a reason for hiding this comment

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

No worries. That diff file is gone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants