fix: make cancel() actually cancel the in-flight query#733
Open
joshmarkovic wants to merge 1 commit into
Open
Conversation
SQLServerConnectionManager.cancel() was a no-op (it only logged), so dbt-core's cancel_open() could not stop sibling queries on Ctrl-C or when another thread failed; in-flight statements kept running server-side, holding locks. Track the in-flight cursor on the Connection during add_query and call Cursor.cancel() on it from cancel(). pyodbc exposes Cursor.cancel() (issuing SQLCancel, designed for cross-thread use) and mssql-python's cursor is used the same way when supported. Cancellation is best-effort: it no-ops when no statement is in flight, the cursor is gone, or the backend cursor lacks cancel(), and it swallows errors from a statement that completed between lookup and cancel. Add unit tests for cancel() (in-flight, absent, unsupported, error paths) and for add_query registering then clearing the cursor.
Collaborator
|
Please check the conflict @joshmarkovic |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
cancel()only wrote a debug log line. It never cancelled anything.When a run is interrupted (Ctrl-C) or one thread hits an error, dbt-core calls
cancel()on the other open connections. Because this adapter'scancel()was a no-op, those queries kept running on the server until they finished on their own.Fix
add_query()now stores the active cursor on the connection while a statement is executing, and always clears it afterwards, including on failure.cancel()looks up that cursor and callsCursor.cancel()on it. pyodbc documents this as safe to call from another thread (it issuesSQLCancel). mssql-python cursors are used the same way when they support it.cancel(), or the call fails because the statement just finished, it logs at debug level and moves on.Testing
add_query()registers the cursor during execution and clears it on both success and failure.