Skip to content

Commit a43e20c

Browse files
committed
Merge remote-tracking branch 'origin/msrathore/sea-retry-backoff' into msrathore/sea-mtls-headers-useragent
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com> # Conflicts: # tests/unit/sea/connectionOptions.test.ts
2 parents 83975cc + 64b9821 commit a43e20c

5 files changed

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

97111
/**
@@ -420,6 +434,33 @@ export function buildSeaHttpOptions(options: ConnectionOptions): SeaHttpOptions
420434
* - `HiveDriverError` for unsupported auth modes / Azure-direct /
421435
* custom persistence / ambiguous combinations.
422436
*/
437+
/**
438+
* Convert the driver's `ClientConfig` retry knobs (milliseconds, total-attempt
439+
* count) into the kernel's `ConnectionOptions` retry kwargs (whole seconds).
440+
* The kernel owns the retry loop on the SEA path, so forwarding these keeps SEA
441+
* and Thrift governed by one retry config. `retryMaxAttempts` is a TOTAL attempt
442+
* count on both sides (the kernel converts to retries-after-first internally),
443+
* so it passes through directly. Sub-second delays round to the nearest second
444+
* (the kernel's granularity); all values are clamped into the napi `u32` range.
445+
*/
446+
export function buildSeaRetryOptions(config: {
447+
retryMaxAttempts: number;
448+
retriesTimeout: number;
449+
retryDelayMin: number;
450+
retryDelayMax: number;
451+
}): Required<
452+
Pick<SeaSessionDefaults, 'retryMinWaitSecs' | 'retryMaxWaitSecs' | 'retryMaxAttempts' | 'retryOverallTimeoutSecs'>
453+
> {
454+
const msToSecs = (ms: number): number => Math.min(MAX_U32, Math.max(0, Math.round(ms / 1000)));
455+
const clampU32 = (n: number): number => Math.min(MAX_U32, Math.max(0, Math.trunc(n)));
456+
return {
457+
retryMinWaitSecs: msToSecs(config.retryDelayMin),
458+
retryMaxWaitSecs: msToSecs(config.retryDelayMax),
459+
retryMaxAttempts: clampU32(config.retryMaxAttempts),
460+
retryOverallTimeoutSecs: msToSecs(config.retriesTimeout),
461+
};
462+
}
463+
423464
export function buildSeaConnectionOptions(options: ConnectionOptions): SeaNativeConnectionOptions {
424465
const { authType } = options as { authType?: string };
425466

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: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,20 @@
1313
// limitations under the License.
1414

1515
import { expect } from 'chai';
16-
import { buildSeaConnectionOptions, buildSeaTlsOptions, buildSeaHttpOptions } from '../../../lib/sea/SeaAuth';
16+
import {
17+
buildSeaConnectionOptions,
18+
buildSeaTlsOptions,
19+
buildSeaHttpOptions,
20+
buildSeaRetryOptions,
21+
} from '../../../lib/sea/SeaAuth';
1722
import { ConnectionOptions } from '../../../lib/contracts/IDBSQLClient';
1823
import HiveDriverError from '../../../lib/errors/HiveDriverError';
1924

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

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

2631
describe('SeaAuth connection options — intervalsAsString default', () => {
2732
it('always sets intervalsAsString:true (thrift-compatible interval rendering)', () => {
@@ -267,5 +272,42 @@ describe('SeaAuth HTTP options (buildSeaHttpOptions)', () => {
267272
) as { customHeaders?: Array<{ name: string; value: string }> };
268273
expect(native.customHeaders?.find((h) => h.name === 'X-Trace')?.value).to.equal('abc');
269274
expect(native.customHeaders?.find((h) => h.name === 'User-Agent')?.value).to.contain('MyApp/2.0');
275+
describe('SeaAuth retry options — buildSeaRetryOptions', () => {
276+
// The driver's ClientConfig retry defaults (ms / total-attempt count).
277+
const defaults = {
278+
retryMaxAttempts: 5,
279+
retriesTimeout: 15 * 60 * 1000,
280+
retryDelayMin: 1000,
281+
retryDelayMax: 60 * 1000,
282+
};
283+
284+
it('converts the connector ms knobs to the kernel whole-second kwargs', () => {
285+
const r = buildSeaRetryOptions(defaults);
286+
expect(r.retryMinWaitSecs).to.equal(1); // 1000ms
287+
expect(r.retryMaxWaitSecs).to.equal(60); // 60000ms
288+
expect(r.retryOverallTimeoutSecs).to.equal(900); // 15min
289+
});
290+
291+
it('passes retryMaxAttempts through as a TOTAL attempt count (kernel converts to retries)', () => {
292+
expect(buildSeaRetryOptions({ ...defaults, retryMaxAttempts: 5 }).retryMaxAttempts).to.equal(5);
293+
expect(buildSeaRetryOptions({ ...defaults, retryMaxAttempts: 0 }).retryMaxAttempts).to.equal(0);
294+
});
295+
296+
it('rounds sub-second delays to the nearest second (kernel granularity)', () => {
297+
const r = buildSeaRetryOptions({ ...defaults, retryDelayMin: 1500, retryDelayMax: 2400 });
298+
expect(r.retryMinWaitSecs).to.equal(2); // 1.5s → 2
299+
expect(r.retryMaxWaitSecs).to.equal(2); // 2.4s → 2
300+
});
301+
302+
it('clamps negative/garbage inputs into the napi u32 range', () => {
303+
const r = buildSeaRetryOptions({
304+
retryMaxAttempts: -3,
305+
retriesTimeout: -1,
306+
retryDelayMin: -1000,
307+
retryDelayMax: 0,
308+
});
309+
expect(r.retryMaxAttempts).to.equal(0);
310+
expect(r.retryMinWaitSecs).to.equal(0);
311+
expect(r.retryOverallTimeoutSecs).to.equal(0);
270312
});
271313
});

0 commit comments

Comments
 (0)