Skip to content

Fail query runner when nodes do not come up #25184

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

Merged
merged 1 commit into from
May 29, 2025

Conversation

tdcmeehan
Copy link
Contributor

Description

When query runners fail to launch a cluster, a log is printed that shows this, but it doesn't fail the query runner. This could result in timeouts. It is better to fail early when the cluster does not launch.

Motivation and Context

More obvious errors.

Impact

See above

Test Plan

Query runners are used throughout our testing suite, so this should be thoroughly tested with existing tests.

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.

Release Notes

== NO RELEASE NOTE ==

@tdcmeehan tdcmeehan requested a review from hantangwangd May 23, 2025 16:25
@prestodb-ci prestodb-ci added the from:IBM PR from IBM label May 23, 2025
@prestodb-ci prestodb-ci requested review from a team, BryanCutler and anandamideShakyan and removed request for a team May 23, 2025 16:25
Copy link
Contributor

@aditi-pandit aditi-pandit left a comment

Choose a reason for hiding this comment

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

Thanks @tdcmeehan for this code.

@tdcmeehan tdcmeehan force-pushed the fail_qr branch 2 times, most recently from 3167687 to 97c0ea1 Compare May 28, 2025 00:53
Copy link
Member

@hantangwangd hantangwangd 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 the fix. Mostly looks good to me, just one nit.

Copy link
Contributor

@BryanCutler BryanCutler left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@hantangwangd hantangwangd left a comment

Choose a reason for hiding this comment

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

LGTM.

@tdcmeehan tdcmeehan merged commit d3f7172 into prestodb:master May 29, 2025
97 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
from:IBM PR from IBM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants