Skip to content

Commit 11f0576

Browse files
committed
[SEA-NodeJS] Remove the dead queryTimeout deadline plumbing (kernel field gone)
Follow-on to making queryTimeout a no-op on SEA: now that the kernel's `query_timeout_secs` field is removed (databricks/databricks-sql-kernel#140) and SeaSessionBackend no longer passes a timeout, the SeaOperationBackend client-side deadline is dead code. Remove it: - `SeaOperationBackend`: drop the `queryTimeoutSecs` option, the `queryTimeoutMs` field, and the async poll-loop deadline enforcement (the `OperationStateError(Timeout)` + best-effort-cancel branch). - Regenerated napi `.d.ts`: `ExecuteOptions` no longer carries `queryTimeoutSecs`. - Tests: remove the obsolete client-side-deadline test; drop the `queryTimeoutSecs` args from the `makeAsyncOp` / `makeSyncOp` helpers. - Comment cleanups in SeaNativeLoader. queryTimeout remains a no-op on SEA (SQL Warehouses use STATEMENT_TIMEOUT). SEA unit suite 261 passing; eslint clean; e2e directResults (CREATE/close/cancel) unaffected. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
1 parent 8f1b268 commit 11f0576

4 files changed

Lines changed: 15 additions & 95 deletions

File tree

lib/sea/SeaNativeLoader.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export type SeaStatement = NativeStatement;
5656
// Per-statement execution options and bound-parameter inputs are kernel
5757
// concerns: the napi binding generates the canonical shapes (`positionalParams`
5858
// / `namedParams` as `TypedValueInput` / `NamedTypedValueInput`, plus
59-
// `rowLimit`, `queryTimeoutSecs`, `statementConf`, `queryTags`). We re-export
59+
// `rowLimit`, `statementConf`, `queryTags`). We re-export
6060
// rather than re-declare so the driver-side param codec can never drift from
6161
// the kernel contract.
6262
export type SeaNativeExecuteOptions = NativeExecuteOptions;

lib/sea/SeaOperationBackend.ts

Lines changed: 3 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -140,15 +140,6 @@ export interface SeaOperationBackendOptions {
140140
* handle exposes one, else a fresh UUIDv4.
141141
*/
142142
id?: string;
143-
/**
144-
* Client-side query timeout in whole seconds (the public `queryTimeout`).
145-
* The kernel ignores `queryTimeoutSecs` on the async submit path
146-
* (`submitStatement` always sends `wait_timeout=0s`), so the JS poll loop
147-
* enforces it as a deadline — on expiry it best-effort cancels the statement
148-
* and throws `OperationStateError(Timeout)`, matching the Thrift path's
149-
* server-side TIMEDOUT outcome. Omitted ⇒ no client-side deadline.
150-
*/
151-
queryTimeoutSecs?: number;
152143
}
153144

154145
export default class SeaOperationBackend implements IOperationBackend {
@@ -190,17 +181,12 @@ export default class SeaOperationBackend implements IOperationBackend {
190181
// already-terminal statement. Drives both fetch and result-metadata.
191182
private fetchHandlePromise?: Promise<SeaFetchHandle>;
192183

193-
// Client-side query-timeout deadline in ms (the public `queryTimeout`),
194-
// undefined when unset. Enforced in the async poll loop.
195-
private readonly queryTimeoutMs?: number;
196-
197184
constructor({
198185
asyncStatement,
199186
statement,
200187
cancellableExecution,
201188
context,
202189
id,
203-
queryTimeoutSecs,
204190
}: SeaOperationBackendOptions) {
205191
// Exactly one of the three handle kinds must be supplied.
206192
const providedCount =
@@ -232,7 +218,6 @@ export default class SeaOperationBackend implements IOperationBackend {
232218
this.context = context;
233219
this._id =
234220
id ?? asyncStatement?.statementId ?? statement?.statementId ?? cancellableExecution?.statementId ?? uuidv4();
235-
this.queryTimeoutMs = queryTimeoutSecs !== undefined && queryTimeoutSecs > 0 ? queryTimeoutSecs * 1000 : undefined;
236221
}
237222

238223
public get id(): string {
@@ -441,9 +426,6 @@ export default class SeaOperationBackend implements IOperationBackend {
441426
if (this.fetchHandlePromise) {
442427
return;
443428
}
444-
// Client-side timeout deadline: the kernel ignores queryTimeoutSecs on the
445-
// async submit path, so we enforce the public `queryTimeout` here.
446-
const deadline = this.queryTimeoutMs !== undefined ? Date.now() + this.queryTimeoutMs : undefined;
447429
for (;;) {
448430
// A JS-initiated cancel/close short-circuits before the next poll.
449431
failIfNotActive(this.lifecycle);
@@ -494,30 +476,8 @@ export default class SeaOperationBackend implements IOperationBackend {
494476
throw new OperationStateError(OperationStateErrorCode.Unknown);
495477
}
496478

497-
// Still Pending/Running — enforce the client-side timeout before sleeping.
498-
if (deadline !== undefined && Date.now() >= deadline) {
499-
// Best-effort server-side cancel so the statement doesn't keep running
500-
// after we stop waiting; never mask the timeout with a cancel failure,
501-
// but warn-log a failed cancel so a still-running server statement is
502-
// diagnosable (mirrors the fetch-error cleanup path).
503-
// eslint-disable-next-line no-await-in-loop
504-
await this.cancel().catch((cancelErr) => {
505-
const cause = cancelErr instanceof Error ? cancelErr.message : String(cancelErr);
506-
this.context
507-
.getLogger()
508-
.log(
509-
LogLevel.warn,
510-
`SEA query-timeout cleanup: cancel() failed for operation ${this._id}; the server-side ` +
511-
`statement may keep running until the session is closed. Cause: ${cause}`,
512-
);
513-
});
514-
// Release the statement handle too (cancel stops the work; close frees
515-
// the handle) — best-effort, mirroring the other terminal branches.
516-
// eslint-disable-next-line no-await-in-loop
517-
await this.bestEffortClose();
518-
throw new OperationStateError(OperationStateErrorCode.Timeout);
519-
}
520-
479+
// Still Pending/Running — sleep before the next poll. (There is no
480+
// client-side query-timeout deadline: `queryTimeout` is a no-op on SEA.)
521481
// eslint-disable-next-line no-await-in-loop
522482
await delay(STATUS_POLL_INTERVAL_MS);
523483
}
@@ -526,7 +486,7 @@ export default class SeaOperationBackend implements IOperationBackend {
526486
/**
527487
* Sync (`runAsync: false`) execute path. Drives the blocking
528488
* `CancellableExecution.result()` to a terminal `Statement` (the kernel polls
529-
* to completion server-side, honouring `queryTimeoutSecs` on this path). The
489+
* to completion server-side). The
530490
* await is interruptible: a JS-initiated `cancel()` fires the detached
531491
* canceller, the server flips the statement terminal, and the parked
532492
* `result()` rejects with `Cancelled` — which we map to the typed

native/sea/index.d.ts

Lines changed: 8 additions & 31 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/unit/sea/execution.test.ts

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -909,9 +909,9 @@ describe('SeaOperationBackend', () => {
909909
});
910910

911911
describe('SeaOperationBackend — async (submitStatement) path', () => {
912-
const makeAsyncOp = (asyncStatement: FakeAsyncStatement, queryTimeoutSecs?: number) =>
912+
const makeAsyncOp = (asyncStatement: FakeAsyncStatement) =>
913913
// eslint-disable-next-line @typescript-eslint/no-explicit-any
914-
new SeaOperationBackend({ asyncStatement: asyncStatement as any, context: makeContext(), queryTimeoutSecs });
914+
new SeaOperationBackend({ asyncStatement: asyncStatement as any, context: makeContext() });
915915

916916
it('rejects when neither asyncStatement nor statement is provided', () => {
917917
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -1011,22 +1011,6 @@ describe('SeaOperationBackend — async (submitStatement) path', () => {
10111011
expect((thrown as OperationStateError).errorCode).to.equal(OperationStateErrorCode.Closed);
10121012
});
10131013

1014-
it('waitUntilReady() enforces queryTimeout client-side: throws Timeout and cancels a stuck Running statement', async function timeoutTest() {
1015-
// eslint-disable-next-line no-invalid-this
1016-
this.timeout(5000);
1017-
const stmt = new FakeAsyncStatement('Running'); // never reaches a terminal state
1018-
const op = makeAsyncOp(stmt, 0.05); // 50ms client-side deadline
1019-
let thrown: unknown;
1020-
try {
1021-
await op.waitUntilReady();
1022-
} catch (err) {
1023-
thrown = err;
1024-
}
1025-
expect(thrown).to.be.instanceOf(OperationStateError);
1026-
expect((thrown as OperationStateError).errorCode).to.equal(OperationStateErrorCode.Timeout);
1027-
// Best-effort server-side cancel fired so the statement doesn't keep running.
1028-
expect(stmt.cancelled).to.equal(true);
1029-
});
10301014

10311015
it('cancel() forwards to the async statement and short-circuits a subsequent poll', async () => {
10321016
const stmt = new FakeAsyncStatement(['Running', 'Running', 'Succeeded']);
@@ -1052,12 +1036,11 @@ describe('SeaOperationBackend — async (submitStatement) path', () => {
10521036
});
10531037

10541038
describe('SeaOperationBackend — sync (executeStatementCancellable) path', () => {
1055-
const makeSyncOp = (cancellableExecution: FakeCancellableExecution, queryTimeoutSecs?: number) =>
1039+
const makeSyncOp = (cancellableExecution: FakeCancellableExecution) =>
10561040
new SeaOperationBackend({
10571041
// eslint-disable-next-line @typescript-eslint/no-explicit-any
10581042
cancellableExecution: cancellableExecution as any,
10591043
context: makeContext(),
1060-
queryTimeoutSecs,
10611044
});
10621045

10631046
it('rejects when more than one handle kind is provided', () => {

0 commit comments

Comments
 (0)