feat(kernel): honor ctx deadlines on connect and mid-fetch#409
Draft
mani-mathur-arch wants to merge 4 commits into
Draft
feat(kernel): honor ctx deadlines on connect and mid-fetch#409mani-mathur-arch wants to merge 4 commits into
mani-mathur-arch wants to merge 4 commits into
Conversation
Wire the kernel backend's two blocking cgo calls to the new cancellable C-ABI entry points via a ctxWatcher that bridges a Go context onto a kernel cancel token: - OpenSession -> kernel_session_open_cancellable - rows.go nextBatch -> kernel_result_stream_next_batch_cancellable Previously each blocked in an uninterruptible cgo call: a slow warehouse cold-start or a hung CloudFetch chunk ignored the caller's ctx deadline. The watcher fires the token on ctx.Done(), which drops the in-flight kernel request future (a real abort), and the call sites prefer the ctx error on cancellation while preserving the kernel error as cause (matching the execute path and the database/sql convention). A session-fatal error is evicted before the ctx-cancelled return so a failure racing a cancel still evicts the conn. A non-cancellable ctx (nil Done) yields a nil watcher -> NULL token -> the plain, unchanged path, so there is zero watcher overhead on the common case. Requires the kernel cancel-token symbols; bump KERNEL_REV to the merged kernel revision before this builds in CI (it links a locally-staged archive today). Tests: tagged ctxWatcher unit tests (fires on cancel/deadline, nil-safe on an uncancellable ctx, clean teardown without fire) exercising the real cgo ctx->token bridge in the build-and-test-kernel CI job. Default CGO_ENABLED=0 build unchanged. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
Bumps the kernel pin from the placeholder statement-surface rev to the tip of the tier2-features branch (databricks-sql-kernel#167 @ 02c0a43), the head of the unmerged kernel PR stack (#165 -> #166 cancel-token -> #167 tier2). It's a linear superset carrying every new C-ABI symbol these driver branches link against: kernel_session_open_cancellable / kernel_result_stream_next_batch_cancellable (cancel token, #166) plus kernel_abi_version, kernel_session_close_blocking, set_tls_client_certificate, set_cloudfetch_enabled (#167). The kernel-lib build fetches the bare commit via its PR-head-ref fallback, so an unmerged SHA is buildable. Temporary: re-pin to the squash-merge SHA once the kernel stack lands. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
mani-mathur-arch
force-pushed
the
mani/sea-kernel-ctx-cancel
branch
from
July 17, 2026 10:07
42af196 to
26cf09b
Compare
mani-mathur-arch
added a commit
that referenced
this pull request
Jul 17, 2026
…etch cancel Address PR #409 review findings: - OpenSession's cancelled-connect path now wraps BOTH the ctx error and the underlying kernel error (two %w), so errors.Is still matches the ctx error while the *KernelError stays reachable via errors.As. Mirrors the execute (operation.go) and next_batch (rows.go) cancelled paths, which already did this; the connect path was dropping the kernel diagnostics. - Add TestKernelE2ECancelDuringFetch, which deterministically reaches rows.Next() after QueryContext succeeds and then observes cancellation during fetch — closing the coverage gap where the read-path cancel could pass without exercising kernel_result_stream_next_batch_cancellable.
…etch cancel Address PR #409 review findings: - OpenSession's cancelled-connect path now wraps BOTH the ctx error and the underlying kernel error (two %w), so errors.Is still matches the ctx error while the *KernelError stays reachable via errors.As. Mirrors the execute (operation.go) and next_batch (rows.go) cancelled paths, which already did this; the connect path was dropping the kernel diagnostics. - Add TestKernelE2ECancelDuringFetch, which deterministically reaches rows.Next() after QueryContext succeeds and then observes cancellation during fetch — closing the coverage gap where the read-path cancel could pass without exercising kernel_result_stream_next_batch_cancellable. Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
mani-mathur-arch
force-pushed
the
mani/sea-kernel-ctx-cancel
branch
from
July 17, 2026 11:12
662aedc to
7c094f8
Compare
The previous TestKernelE2ECancelDuringFetch cancelled between batches, so nextBatch's pre-fetch ctx.Err() guard short-circuited before ever calling kernel_result_stream_next_batch_cancellable — it only proved the read path honored ctx, not the in-flight token abort. Rework it to cancel while a fetch is actually blocked: a watcher goroutine fires the cancel only once a single rows.Next() has been blocked longer than any buffered-row return could take (i.e. it is inside the network fetch). The run is verified to reach the cancellable call by the "next_batch cancelled" wrapper the C-call error path emits (the pre-fetch guard returns a bare context error); attempts are retried a bounded number of times to absorb the timing window. Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
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
1 participant
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.
What
Wire the kernel backend's two blocking cgo calls to the new cancellable C-ABI entry points via a
ctxWatcherthat bridges a Gocontextonto a kernel cancel token:OpenSession→kernel_session_open_cancellable(connect)rows.go nextBatch→kernel_result_stream_next_batch_cancellable(mid-fetch)Previously each blocked in an uninterruptible cgo call: a slow warehouse cold-start or a hung CloudFetch chunk ignored the caller's ctx deadline. The watcher fires the token on
ctx.Done(), dropping the in-flight kernel request future (a real abort); the call sites prefer the ctx error while preserving the kernel error as cause, and evict a session-fatal conn before the ctx-cancelled return. A non-cancellable ctx yields a nil watcher → NULL token → the plain, unchanged path (zero overhead).CloseSessionstays fire-and-forget.Closes PECOBLR-3645 + the connect half of PECOBLR-3646.
Tests
ctxWatcherunit tests (fires on cancel/deadline, nil-safe on an uncancellable ctx, clean teardown) exercising the real cgo ctx→token bridge in thebuild-and-test-kernelCI job.context.DeadlineExceeded. DefaultCGO_ENABLED=0build unchanged.Co-authored-by: Isaac