[SEA-NodeJS] Forward retry/backoff tuning to the kernel on the SEA path - #427
Closed
msrathore-db wants to merge 1 commit into
Closed
[SEA-NodeJS] Forward retry/backoff tuning to the kernel on the SEA path#427msrathore-db wants to merge 1 commit into
msrathore-db wants to merge 1 commit into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
The kernel owns the retry loop on the SEA/use_kernel path, so forward the driver's existing ClientConfig retry knobs (the same ones the Thrift HttpRetryPolicy reads) onto the napi ConnectionOptions retry kwargs — keeping SEA and Thrift governed by one retry config. Mirrors Python connector #820. - buildSeaRetryOptions(config): ms -> whole seconds, clamped to napi u32. retryDelayMin->retryMinWaitSecs, retryDelayMax->retryMaxWaitSecs, retriesTimeout->retryOverallTimeoutSecs, retryMaxAttempts passes through as a TOTAL attempt count (the kernel converts to retries-after-first). - SeaBackend.connect() merges it into the native options from the client config. - Adds SeaSessionDefaults retry fields + unit tests (mapping, rounding, clamp). Requires kernel napi retry kwargs (databricks-sql-kernel #141). KERNEL_REV is pinned to #141's branch HEAD as a placeholder — MUST be re-pinned to #141's squash-merge SHA before this merges (orphan-SHA risk otherwise). Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@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
The kernel owns the retry loop on the SEA (useSEA/use_kernel) path, so this forwards the driver's existing ClientConfig retry knobs — the same ones the Thrift HttpRetryPolicy reads — onto the napi ConnectionOptions retry kwargs. SEA and Thrift are then governed by one retry config. This is the Node analogue of Python connector #820.
Changes
buildSeaRetryOptions(config) (lib/sea/SeaAuth.ts) — converts the connector's milliseconds → the kernel's whole seconds, clamped into the napi u32 range:
SeaBackend.connect() merges the retry options (read from context.getConfig()) into the native options.
SeaSessionDefaults gains the four optional retry fields; unit tests cover the mapping, sub-second rounding, and the negative/garbage clamp.
retryMaxAttempts passes through directly because Node's policy (attempt >= retryMaxAttempts) already treats it as a total attempt count, matching the kernel semantics. The default knobs (1s/60s/900s) line up with the kernel defaults.
⚠️ Blocked on kernel #141
Depends on the napi retry kwargs added in databricks-sql-kernel #141. KERNEL_REV is pinned to #141's branch HEAD as a placeholder — must be re-pinned to #141's squash-merge SHA before this merges (orphan-SHA risk otherwise). Draft until #141 lands.
This pull request and its description were written by Isaac.