Repository navigation
fix: report a close or cancel during a wait as OperationStateError - #609
Open
vuanhphung wants to merge 2 commits into
Open
vuanhphung wants to merge 2 commits into
vuanhphung wants to merge 2 commits into
Conversation
Closing or cancelling an operation while fetchAll(), fetchChunk() or
finished() waits on it could reject the wait with the server's answer
for the released statement. On Thrift, closing the session while a query
runs makes the in-flight GetOperationStatus return HTTP 409 ("A concurrent
GetOperationStatus request updated the command. Please retry."), which
surfaced as a THTTPException. DBSQLOperation now records a close or cancel
from the moment it is requested and reports such failures as
OperationStateError CLOSED or CANCELED.
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
vuanhphung
had a problem deploying
to
azure-prod
November 13, 2026 08:35 — with
GitHub Actions
Failure
vuanhphung
temporarily deployed
to
azure-prod
November 13, 2026 08:35 — with
GitHub Actions
Inactive
vuanhphung
temporarily deployed
to
azure-prod
November 13, 2026 08:35 — with
GitHub Actions
Inactive
There was a problem hiding this comment.
Verdict: 1 Low
Looks good overall. The new close/cancel flags fix the 409 case as described, and the tests cover the main paths. One low concern: if the in-flight request fails before a close or cancel that then fails, the waiter is told CLOSED/CANCELED while the operation stays open, and the original error is dropped without a log. That case isn't tested.
vuanhphung
temporarily deployed
to
azure-prod
November 13, 2026 09:49 — with
GitHub Actions
Inactive
A status poll or result fetch could fail while the close or cancel that caused it was still pending. The waiter was then told CLOSED or CANCELED even if that request went on to fail, leaving the operation open and dropping the original error. Keep the latest close/cancel request and await it before choosing the error: report the client's action only if it succeeded, otherwise rethrow the original error. Signed-off-by: Vu Anh Phung <vu.phung@databricks.com> Co-authored-by: Isaac <no-reply@databricks.com>
vuanhphung
temporarily deployed
to
azure-prod
November 13, 2026 09:54 — with
GitHub Actions
Inactive
vuanhphung
temporarily deployed
to
azure-prod
November 13, 2026 09:54 — with
GitHub Actions
Inactive
vuanhphung
temporarily deployed
to
azure-prod
November 13, 2026 09:54 — with
GitHub Actions
Inactive
This branch was previously deployed
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
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.
Closing a session while one of its queries is running should make the code waiting on that query (
fetchAll(),fetchChunk(),finished()) fail with a closed error. On Thrift it often fails with the transport error instead:session.close()sendsCloseOperationwhile the wait'sGetOperationStatusis still in flight. The server then answers that poll with HTTP 409 about two times in three, andDBSQLOperationpasses the error through, even though the client itself closed the operation. The "Please retry" text points callers the wrong way. The databricks-driver-test CONCURRENT-004 test (databricks/databricks-driver-test#2082) reproduced this in 5 of 7 live Thrift runs againstmain(the proxy recorded the 409 on the poll). In the other runs the poll got HTTP 200 and the driver already reportedThe operation was closed by a client. A cancel at the same point already getsCANCELED_STATEback, and SEA already reported a closed error.DBSQLOperationnow keeps the latest close or cancel request. When a wait or fetch fails with anything other than anOperationStateError, it waits for that request to settle. If the close or cancel succeeded, it reportsOperationStateErrorCLOSEDorCANCELED. If it failed, or there was none, it rethrows the original error.Verification:
tests/unit/DBSQLOperation.test.ts. Three fail without the change, and three cover the cases that must keep the original error, including a close that fails after the poll did.npm test(1757 passing),npm run lint,npm run type-checkand prettier are clean.This pull request and its description were written by Isaac.