Skip to content

feat(kernel): add client query timeout - #484

Open
cathleeny wants to merge 1 commit into
mainfrom
feat/client-query-timeout
Open

cathleeny wants to merge 1 commit into
mainfrom
feat/client-query-timeout

Conversation

@cathleeny

@cathleeny cathleeny commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add connector-only WithClientQueryTimeout for the SEA/kernel backend.
  • Positive values use kernel_statement_execute_with_timeout_ms; zero and maximum time.Duration are unlimited.
  • Bound execution through terminal-state mapping, excluding result materialization/fetching. WithTimeout remains the independent server-side Thrift option.
  • Keep context-cancel cleanup from extending an explicit client deadline while preserving the legacy execute cleanup contract.

Depends on databricks-sql-kernel#339 at ec639bd48e84c64d708a0b70a532e44ecf83ad7c; updated kernel bindings are required before release.

Validation

  • Kernel-tagged suite and race test against the exact PR 339 archive
  • Live E2E: returned non-retryable HYT00 in 5.002s, then successfully reused the same sql.Conn for SELECT 1

Signed-off-by: cat23123 <58714163+cat23123@users.noreply.github.com>
@cathleeny
cathleeny marked this pull request as ready for review September 15, 2026 20:01

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: 1 Medium

Looks solid — thorough tests and documentation for the new WithClientQueryTimeout kernel option; config plumbing, DeepCopy, sentinel wiring, and the C-ABI max constant are all correct and covered. One medium concurrency concern: the detached-watcher path inverts the legacy canceller-before-statement teardown ordering, whose safety rests on a kernel contract not verifiable from this repo.

Comment thread internal/backend/kernel/operation.go
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.

1 participant