Skip to content

Commit 64b9821

Browse files
committed
feat(sea): forward retry/backoff tuning to the kernel on the SEA path
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>
1 parent f4d1675 commit 64b9821

5 files changed

Lines changed: 121 additions & 5 deletions

File tree

KERNEL_REV

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
80b68e1eef3b613910183a50dfa4dace854d50dd
1+
fcc459bbf3f39bf57e2ee02f14b99c0ec7a70123

lib/sea/SeaAuth.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,20 @@ export interface SeaSessionDefaults {
9191
* integer within the napi `u32` range by `buildSeaConnectionOptions`.
9292
*/
9393
maxConnections?: number;
94+
/**
95+
* Retry/backoff tuning forwarded to the kernel (which owns the retry loop
96+
* on the SEA path). These mirror the driver's `ClientConfig` retry knobs —
97+
* the same ones the Thrift `HttpRetryPolicy` uses — converted from the
98+
* connector's milliseconds to the kernel's whole seconds, so a single
99+
* retry config governs both backends. Unset ⇒ kernel default policy.
100+
* Map onto the napi `ConnectionOptions.retry{Min,Max}WaitSecs` /
101+
* `retryMaxAttempts` / `retryOverallTimeoutSecs` (see `buildSeaRetryOptions`).
102+
*/
103+
retryMinWaitSecs?: number;
104+
retryMaxWaitSecs?: number;
105+
/** **Total** attempts (kernel converts to retries-after-first internally). */
106+
retryMaxAttempts?: number;
107+
retryOverallTimeoutSecs?: number;
94108
}
95109

96110
/**
@@ -274,6 +288,33 @@ export function buildSeaTlsOptions(options: ConnectionOptions): SeaTlsOptions {
274288
* - `HiveDriverError` for unsupported auth modes / Azure-direct /
275289
* custom persistence / ambiguous combinations.
276290
*/
291+
/**
292+
* Convert the driver's `ClientConfig` retry knobs (milliseconds, total-attempt
293+
* count) into the kernel's `ConnectionOptions` retry kwargs (whole seconds).
294+
* The kernel owns the retry loop on the SEA path, so forwarding these keeps SEA
295+
* and Thrift governed by one retry config. `retryMaxAttempts` is a TOTAL attempt
296+
* count on both sides (the kernel converts to retries-after-first internally),
297+
* so it passes through directly. Sub-second delays round to the nearest second
298+
* (the kernel's granularity); all values are clamped into the napi `u32` range.
299+
*/
300+
export function buildSeaRetryOptions(config: {
301+
retryMaxAttempts: number;
302+
retriesTimeout: number;
303+
retryDelayMin: number;
304+
retryDelayMax: number;
305+
}): Required<
306+
Pick<SeaSessionDefaults, 'retryMinWaitSecs' | 'retryMaxWaitSecs' | 'retryMaxAttempts' | 'retryOverallTimeoutSecs'>
307+
> {
308+
const msToSecs = (ms: number): number => Math.min(MAX_U32, Math.max(0, Math.round(ms / 1000)));
309+
const clampU32 = (n: number): number => Math.min(MAX_U32, Math.max(0, Math.trunc(n)));
310+
return {
311+
retryMinWaitSecs: msToSecs(config.retryDelayMin),
312+
retryMaxWaitSecs: msToSecs(config.retryDelayMax),
313+
retryMaxAttempts: clampU32(config.retryMaxAttempts),
314+
retryOverallTimeoutSecs: msToSecs(config.retriesTimeout),
315+
};
316+
}
317+
277318
export function buildSeaConnectionOptions(options: ConnectionOptions): SeaNativeConnectionOptions {
278319
const { authType } = options as { authType?: string };
279320

lib/sea/SeaBackend.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { LogLevel } from '../contracts/IDBSQLLogger';
2121
import HiveDriverError from '../errors/HiveDriverError';
2222
import { getSeaNative, SeaNativeBinding, SeaConnection } from './SeaNativeLoader';
2323
import { decodeNapiKernelError } from './SeaErrorMapping';
24-
import { buildSeaConnectionOptions, SeaNativeConnectionOptions } from './SeaAuth';
24+
import { buildSeaConnectionOptions, buildSeaRetryOptions, SeaNativeConnectionOptions } from './SeaAuth';
2525
import { installKernelLogBridge } from './SeaLogging';
2626
import SeaSessionBackend from './SeaSessionBackend';
2727

@@ -85,7 +85,14 @@ export default class SeaBackend implements IBackend {
8585
// Validate PAT auth + capture the napi-binding option shape.
8686
// Any non-PAT mode (or a missing/empty token) throws here, before
8787
// we ever touch the native binding.
88-
this.nativeOptions = buildSeaConnectionOptions(options);
88+
// Forward the driver's retry config to the kernel, which owns the retry
89+
// loop on the SEA path. This keeps SEA and Thrift governed by one retry
90+
// config (the same `ClientConfig` knobs the Thrift `HttpRetryPolicy` reads),
91+
// converted from the connector's milliseconds to the kernel's whole seconds.
92+
this.nativeOptions = {
93+
...buildSeaConnectionOptions(options),
94+
...buildSeaRetryOptions(this.context.getConfig()),
95+
};
8996

9097
// Bridge the Rust kernel's `tracing` logs into the SAME `DBSQLLogger` the
9198
// driver logs through, so logs from all three layers (driver, napi shim,

native/sea/index.d.ts

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

tests/unit/sea/connectionOptions.test.ts

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,15 @@
1313
// limitations under the License.
1414

1515
import { expect } from 'chai';
16-
import { buildSeaConnectionOptions, buildSeaTlsOptions } from '../../../lib/sea/SeaAuth';
16+
import { buildSeaConnectionOptions, buildSeaTlsOptions, buildSeaRetryOptions } from '../../../lib/sea/SeaAuth';
1717
import { ConnectionOptions } from '../../../lib/contracts/IDBSQLClient';
1818
import HiveDriverError from '../../../lib/errors/HiveDriverError';
1919

2020
const PAT = { host: 'h.databricks.com', path: '/sql/1.0/warehouses/abc', token: 'dapi-x' };
2121

2222
// Cast helper: the SEA connection-tuning/TLS options live on the internal
2323
// surface, so tests build untyped option literals.
24-
const opts = (extra: Record<string, unknown>) => ({ ...PAT, ...extra } as unknown as ConnectionOptions);
24+
const opts = (extra: Record<string, unknown>) => ({ ...PAT, ...extra }) as unknown as ConnectionOptions;
2525

2626
describe('SeaAuth connection options — intervalsAsString default', () => {
2727
it('always sets intervalsAsString:true (thrift-compatible interval rendering)', () => {
@@ -119,3 +119,43 @@ describe('SeaAuth TLS options (buildSeaTlsOptions)', () => {
119119
expect(native.checkServerCertificate).to.equal(false);
120120
});
121121
});
122+
123+
describe('SeaAuth retry options — buildSeaRetryOptions', () => {
124+
// The driver's ClientConfig retry defaults (ms / total-attempt count).
125+
const defaults = {
126+
retryMaxAttempts: 5,
127+
retriesTimeout: 15 * 60 * 1000,
128+
retryDelayMin: 1000,
129+
retryDelayMax: 60 * 1000,
130+
};
131+
132+
it('converts the connector ms knobs to the kernel whole-second kwargs', () => {
133+
const r = buildSeaRetryOptions(defaults);
134+
expect(r.retryMinWaitSecs).to.equal(1); // 1000ms
135+
expect(r.retryMaxWaitSecs).to.equal(60); // 60000ms
136+
expect(r.retryOverallTimeoutSecs).to.equal(900); // 15min
137+
});
138+
139+
it('passes retryMaxAttempts through as a TOTAL attempt count (kernel converts to retries)', () => {
140+
expect(buildSeaRetryOptions({ ...defaults, retryMaxAttempts: 5 }).retryMaxAttempts).to.equal(5);
141+
expect(buildSeaRetryOptions({ ...defaults, retryMaxAttempts: 0 }).retryMaxAttempts).to.equal(0);
142+
});
143+
144+
it('rounds sub-second delays to the nearest second (kernel granularity)', () => {
145+
const r = buildSeaRetryOptions({ ...defaults, retryDelayMin: 1500, retryDelayMax: 2400 });
146+
expect(r.retryMinWaitSecs).to.equal(2); // 1.5s → 2
147+
expect(r.retryMaxWaitSecs).to.equal(2); // 2.4s → 2
148+
});
149+
150+
it('clamps negative/garbage inputs into the napi u32 range', () => {
151+
const r = buildSeaRetryOptions({
152+
retryMaxAttempts: -3,
153+
retriesTimeout: -1,
154+
retryDelayMin: -1000,
155+
retryDelayMax: 0,
156+
});
157+
expect(r.retryMaxAttempts).to.equal(0);
158+
expect(r.retryMinWaitSecs).to.equal(0);
159+
expect(r.retryOverallTimeoutSecs).to.equal(0);
160+
});
161+
});

0 commit comments

Comments
 (0)