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

[ADAP-879] [Feature] Query Cancellation #917

Closed
3 tasks done
d-cole opened this issue Sep 8, 2023 · 4 comments · Fixed by #1251
Closed
3 tasks done

[ADAP-879] [Feature] Query Cancellation #917

d-cole opened this issue Sep 8, 2023 · 4 comments · Fixed by #1251
Labels
enhancement New feature or request

Comments

@d-cole
Copy link
Contributor

d-cole commented Sep 8, 2023

Is this your first time submitting a feature request?

  • I have read the expectations for open source contributors
  • I have searched the existing issues, and I could not find an existing issue for this feature
  • I am requesting a straightforward extension of existing dbt-bigquery functionality, rather than a Big Idea better suited to a discussion

Describe the feature

When a dbt-bigquery run is interrupted running queries should be canceled.

Describe alternatives you've considered

Relying on timeouts possibly added by this PR. While timeouts can help, it does not solve the issue of wasted resources for local query cancelation.

Who will this benefit?

This will benefit users of dbt-bigquery in two scenarios:

  • When executions triggered during local development are interrupted, this feature will avoid wasting compute. This will help reduce cost, especially when users are using on-demand billing. Additionally, it will save users time as they will not need to use the BQ ui and api to identify and cancel query jobs.

  • This will allow for orchestrator-configured timeouts (e.g. airflow) to safely cancel model execution. This will save compute and ensure retries don't rerun a query that was unknowingly completed or still running.

Are you interested in contributing this feature?

Yes #918

Anything else?

No response

@d-cole d-cole added enhancement New feature or request triage labels Sep 8, 2023
@github-actions github-actions bot changed the title [Feature] Query Cancellation [ADAP-879] [Feature] Query Cancellation Sep 8, 2023
@d-cole d-cole mentioned this issue Sep 8, 2023
4 tasks
@github-christophe-oudar
Copy link
Contributor

Great feature 👍
One thing I'd like to see added on top of the job_id set up is to be able to override that id generation with a Jinja macro.
Creating the id (with the UUID) and then passing to the Jinja to transform it, would enable to prefix/suffix the job_id with dbt_ for instance (to easily spot dbt jobs) or even further add the model name!
WDYT?

@colin-rogers-dbt
Copy link
Contributor

Ideally this could be handled in dbt-core, have opened this discussion: dbt-labs/dbt-core#8522

@d-cole
Copy link
Contributor Author

d-cole commented Sep 19, 2023

Ideally this could be handled in dbt-core, have opened this discussion: dbt-labs/dbt-core#8522

@colin-rogers-dbt
Is it possible to prioritize adding cancellation in dbt-bigquery before a standard approach is implemented in core as mentioned here? I'm concerned landing on a standard implementation in core may not get prioritized or be practical for a few reasons:

  1. BaseConnectionManager.cancel_open and SQLConnectionManager.cancel can be viewed as an existing standard.

  2. Defining a standard in core that can generalize well to the many adapters may not be feasible due differences in the adapters. For example, snowflake doesn't use a model identifier on cancelation and cancels all queries in the session. Similarly, Databricks and dbt-trino just call cancel on the cursor.

If that standard is not finalized yet, I'd also be happy to chime in and follow along in its design, eventually working it back into the implementation proposed in this PR. Thank you for your time and for considering this issue and PR!

@guanwang92
Copy link

I recently encountered this issue and I'm really excited about it. I'm curious to know what the roadmap is for addressing this.

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

Successfully merging a pull request may close this issue.

5 participants