Skip to content

feat(sea): type async-execute napi surface + pecotesting e2e#394

Open
msrathore-db wants to merge 3 commits into
msrathore/sea-statement-optionsfrom
msrathore/sea-async-execute
Open

feat(sea): type async-execute napi surface + pecotesting e2e#394
msrathore-db wants to merge 3 commits into
msrathore/sea-statement-optionsfrom
msrathore/sea-async-execute

Conversation

@msrathore-db
Copy link
Copy Markdown
Contributor

Summary

JS-side typing + live e2e test for the new napi async-execute surface delivered by kernel branch msrathore/krn-async-execute (commit 1957878, PR https://github.com/databricks/databricks-sql-kernel/pull/76).

Plumbing

  • SeaNativeLoader.ts — new typed interfaces:

    • SeaNativeStatementStatus — string union mirroring the kernel StatementStatus variant names ('Pending' | 'Running' | 'Succeeded' | 'Failed' | 'Cancelled' | 'Closed' | 'Unknown')
    • SeaNativeAsyncStatementstatementId, status(), awaitResult(), cancel(), close()
    • SeaNativeAsyncResultHandlestatementId, fetchNextBatch(), schema() (no cancel/close — parent owns lifecycle)
    • SeaNativeConnection.submitStatement(sql, options?) method
  • native/sea/index.d.ts — mirrors the napi-rs codegen: new AsyncStatement / AsyncResultHandle class declarations + Connection.submitStatement(...) method.

  • tests/e2e/sea/async-execute-e2e.test.ts — three live-warehouse cases:

    1. submit → awaitResult → drain 100 rows from range(0, 100); byte-decode the Arrow IPC and assert row count
    2. status() returns a string variant from the kernel enum
    3. cancel() against a still-pending statement (range(0, 100_000_000)) completes within 2s wire-latency budget

Deferred (follow-on PR)

A full SeaAsyncOperationBackend implementing IOperationBackend so DBSQLOperation async-mode flows through the same facade callers use for Thrift. The kernel + napi surface is ready; the facade-level integration is the next increment. This PR ships the kernel→napi→JS shape and locks the behaviour with a live e2e.

ULTRATHINK on async-cancel safety

Kernel's AwaitResultCancelGuard (src/statement/executed_async.rs:305-356) installs an RAII drop hook in await_result() that fires a fire-and-forget cancel_statement if the future is dropped mid-poll. This covers:

  • JS Promise.race losers
  • AbortController.abort()-driven cancellation
  • timeout wrappers (Promise.race([awaitResult(), timer]))
  • V8 GC of an orphaned Promise

The napi util::guarded catch_unwind covers V8 panic-across-boundary on top. The Arc<Mutex<Option<…>>> lock shape in the napi AsyncStatement wrapper matches the existing Connection and Statement so cancel/close queue behind any in-flight awaitResult() until it returns naturally — the kernel handles the drop-the-future case independently.

Cross-repo dependency

Requires kernel branch msrathore/krn-async-execute (PR https://github.com/databricks/databricks-sql-kernel/pull/76). Stacks on msrathore/sea-statement-options (F3 + F4, PR #393).

Test plan

  • tsc shows zero new TS errors (14 baseline preserved)
  • Tests written against pecotesting env-var gate (skipped when absent)
  • Live warehouse run — pending merge of kernel PR so the native binary can be built locally

@msrathore-db msrathore-db force-pushed the msrathore/sea-statement-options branch from 1056dfa to 4e673fd Compare May 25, 2026 22:33
@msrathore-db msrathore-db force-pushed the msrathore/sea-async-execute branch from 2538c19 to e314073 Compare May 25, 2026 22:33
@msrathore-db msrathore-db force-pushed the msrathore/sea-statement-options branch from 4e673fd to 7d07de3 Compare May 25, 2026 22:38
@msrathore-db msrathore-db force-pushed the msrathore/sea-async-execute branch from e314073 to e21a4bb Compare May 25, 2026 22:38
@msrathore-db msrathore-db force-pushed the msrathore/sea-statement-options branch from 7d07de3 to 8c5618c Compare May 25, 2026 22:50
@msrathore-db msrathore-db force-pushed the msrathore/sea-async-execute branch from e21a4bb to ca04a30 Compare May 25, 2026 22:51
Mirrors the new napi `submitStatement` / `AsyncStatement` /
`AsyncResultHandle` shapes from kernel branch
`msrathore/krn-async-execute` (commit `1957878`) on the JS side
and adds an end-to-end test against pecotesting.

Plumbing:
- `SeaNativeLoader.ts` — new typed interfaces
  `SeaNativeStatementStatus` (string union mirroring the kernel
  `StatementStatus` variant names), `SeaNativeAsyncStatement`,
  `SeaNativeAsyncResultHandle`. `SeaNativeConnection` gains the
  `submitStatement(sql, options?)` method.
- `native/sea/index.d.ts` — adds `AsyncStatement` /
  `AsyncResultHandle` class declarations and the
  `Connection.submitStatement(...)` method. Mirrors the napi-rs
  codegen exactly.
- `tests/e2e/sea/async-execute-e2e.test.ts` — three cases:
  1. submit → awaitResult → drain 100 rows; assert byte-decoded
     Arrow row count matches the range(0, 100) query.
  2. status() returns a string variant from the kernel enum
     (Pending/Running/Succeeded/Closed depending on warehouse
     responsiveness).
  3. cancel() against a still-pending statement (range(0,
     100_000_000)) completes within a 2s wire-latency budget.

Deferred to a follow-on PR: a full `SeaAsyncOperationBackend`
implementing the facade's `IOperationBackend` interface so
`DBSQLOperation` async-mode (matching Thrift's
`IDBSQLOperation` polling-mode surface) flows transparently.
The kernel + napi surface is ready; the JS adapter is the next
increment.

Cross-PR dependency: requires kernel branch
`msrathore/krn-async-execute` (commit `1957878`). Stacks on
`msrathore/sea-statement-options` (F3 + F4).

Co-authored-by: Isaac
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
Same skip-gate fix as F2/F4 (DA round 1 H1):
- `before()` verifies `native/sea/index.linux-x64-gnu.node` exists,
  skips with a clear "run yarn build:native" message if absent.
- `loadBinding()` uses `createRequire(import.meta.url)` so the
  require works under both CJS and the ESM-reparse path mocha 11+
  may use (MODULE_TYPELESS_PACKAGE_JSON reparse-as-ESM).

Pre-emptive fix even though F3b wasn't on the DA fixup batch list —
the same defect pattern would have been flagged on the next round.

Co-authored-by: Isaac
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
Same ESM-reparse fix as F2/F4. Verified live e2e passes (submit
→ awaitResult → drain, status() variant, cancel-while-pending).

Co-authored-by: Isaac
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
@msrathore-db msrathore-db force-pushed the msrathore/sea-async-execute branch from ca04a30 to de55b96 Compare May 25, 2026 23:20
@msrathore-db
Copy link
Copy Markdown
Contributor Author

Re-review requested — F1-F4 stack underneath this PR is now DA-approved at round-3; this PR (F3b NodeJS async-execute) is the next adversarial-review target.

Round-3 final tip: de55b96f268410269c90666921ece6deef7bdff5

This PR was parked behind the F1-F4 quality bar per team-lead's hard-stop directive. With F1-F4 + matrix now in APPROVED FOR MERGE state, F3b adversarial review starts on this PR + the kernel companion (#76).

Findings will be addressed per the same protocol that closed F1-F4 round-1 → round-2 → round-3:

  • ground-truth origin state before re-implementing,
  • gate exit codes pasted on every fixup notification (eslint, tsc --noEmit, live e2e),
  • DA verifies each finding independently in their check-worktree.

DA preview notes on F3b (Q1 IPC encoder lift to bindings_common, Q2 typed-status fallback handling, Q3 progress-UI error-envelope path, Q4 keep AsyncResultHandle without close, Q5 soften throughput claim, Q6 facade-integration deferral) are stashed for the formal review.

Co-authored-by: Isaac

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