Skip to content

Commit c5a50fc

Browse files
committed
feat: make retry-policy knobs configurable via connect() (both backends)
`retryMaxAttempts` / `retriesTimeout` / `retryDelayMin` / `retryDelayMax` lived only in the internal `ClientConfig` (with defaults 5 / 15min / 1s / 60s) and were consumed by the Thrift `HttpRetryPolicy` and forwarded to the kernel on the SEA path (#420) — but `connect()` never ingested them from user options, and they were not declared on `ConnectionOptions`. So a caller setting `retryMaxAttempts` had no effect on either backend; the hardcoded defaults always applied. Declare the four knobs on `ConnectionOptions` and copy them into `ClientConfig` in `connect()` (per-key narrowed copy, matching the metricView / precision / telemetry knobs). Because both backends already read `ClientConfig` — Thrift via `HttpRetryPolicy.getConfig()`, SEA via `buildKernelRetryOptions(getConfig())` — this one change makes the knobs honored on Thrift and SEA simultaneously, at parity. Semantics are total-attempts on both backends: `retryMaxAttempts=N` caps at N total attempts (initial + retries); `0`/`1` both mean a single attempt. Verified end-to-end on SEA against a persistent-429 fault: `retryMaxAttempts=2` → 2 attempts, `=0` → 1 attempt (previously both produced the default 5). Thrift's `HttpRetryPolicy` increments-then-checks `attempt >= retryMaxAttempts`, giving the same total-attempt count. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
1 parent 93b0331 commit c5a50fc

3 files changed

Lines changed: 77 additions & 0 deletions

File tree

lib/DBSQLClient.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -611,6 +611,24 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I
611611
this.config.preserveBigNumericPrecision = options.preserveBigNumericPrecision;
612612
}
613613

614+
// Retry-policy knobs. These live in ClientConfig (consumed by the Thrift
615+
// HttpRetryPolicy and forwarded to the kernel on the SEA path), so copying
616+
// them here makes them configurable from connect() on BOTH backends. A
617+
// per-key narrowed copy (not a spread) keeps the structural type system
618+
// honest, matching the metricView / precision / telemetry knobs above.
619+
if (options.retryMaxAttempts !== undefined) {
620+
this.config.retryMaxAttempts = options.retryMaxAttempts;
621+
}
622+
if (options.retriesTimeout !== undefined) {
623+
this.config.retriesTimeout = options.retriesTimeout;
624+
}
625+
if (options.retryDelayMin !== undefined) {
626+
this.config.retryDelayMin = options.retryDelayMin;
627+
}
628+
if (options.retryDelayMax !== undefined) {
629+
this.config.retryDelayMax = options.retryDelayMax;
630+
}
631+
614632
// Override telemetry config if provided in options. Per-key narrowed copy
615633
// preserves the structural type system: `ConnectionOptions` and
616634
// `ClientConfig` declare identical types for these knobs, so a user

lib/contracts/IDBSQLClient.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,29 @@ export type ConnectionOptions = {
5959
proxy?: ProxyOptions;
6060
enableMetricViewMetadata?: boolean;
6161

62+
/**
63+
* Retry-policy knobs governing how the driver retries retryable requests.
64+
* They apply to **both** backends: the Thrift `HttpRetryPolicy` reads them
65+
* directly, and on the kernel (SEA) path they are forwarded to the kernel
66+
* (which owns the retry loop) via `buildKernelRetryOptions`. An unset field
67+
* keeps the driver default shown below.
68+
*
69+
* • `retryMaxAttempts` — maximum TOTAL number of attempts (the initial
70+
* request plus any retries). Default 5. `0` or `1` both mean a single
71+
* attempt with no retry. Both backends honour the same total-attempt
72+
* semantics (the kernel converts it to its after-initial retry count).
73+
* • `retriesTimeout` — maximum total wallclock spent retrying, in
74+
* milliseconds. Default 900000 (15 minutes).
75+
* • `retryDelayMin` — minimum backoff between attempts, in milliseconds.
76+
* Default 1000.
77+
* • `retryDelayMax` — maximum backoff between attempts, in milliseconds.
78+
* Default 60000.
79+
*/
80+
retryMaxAttempts?: number;
81+
retriesTimeout?: number;
82+
retryDelayMin?: number;
83+
retryDelayMax?: number;
84+
6285
/**
6386
* Preserve full numeric precision in results. When `true`, DECIMAL columns
6487
* are returned as exact strings and 64-bit integers (BIGINT) as JS `bigint`,

tests/unit/DBSQLClient.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -620,6 +620,42 @@ describe('DBSQLClient.createAuthProvider', () => {
620620
});
621621
});
622622

623+
describe('DBSQLClient retry-policy ConnectionOptions', () => {
624+
it('ingests retry-policy options from connect() into ClientConfig', async () => {
625+
const client = new DBSQLClient();
626+
627+
// Defaults before connect.
628+
expect(client.getConfig().retryMaxAttempts).to.equal(5);
629+
expect(client.getConfig().retriesTimeout).to.equal(15 * 60 * 1000);
630+
expect(client.getConfig().retryDelayMin).to.equal(1000);
631+
expect(client.getConfig().retryDelayMax).to.equal(60 * 1000);
632+
633+
await client.connect({
634+
...connectOptions,
635+
retryMaxAttempts: 2,
636+
retriesTimeout: 5000,
637+
retryDelayMin: 100,
638+
retryDelayMax: 500,
639+
});
640+
641+
expect(client.getConfig().retryMaxAttempts).to.equal(2);
642+
expect(client.getConfig().retriesTimeout).to.equal(5000);
643+
expect(client.getConfig().retryDelayMin).to.equal(100);
644+
expect(client.getConfig().retryDelayMax).to.equal(500);
645+
});
646+
647+
it('keeps defaults when retry-policy options are omitted', async () => {
648+
const client = new DBSQLClient();
649+
650+
await client.connect({ ...connectOptions });
651+
652+
expect(client.getConfig().retryMaxAttempts).to.equal(5);
653+
expect(client.getConfig().retriesTimeout).to.equal(15 * 60 * 1000);
654+
expect(client.getConfig().retryDelayMin).to.equal(1000);
655+
expect(client.getConfig().retryDelayMax).to.equal(60 * 1000);
656+
});
657+
});
658+
623659
describe('DBSQLClient.enableMetricViewMetadata', () => {
624660
it('should store enableMetricViewMetadata config when enabled', async () => {
625661
const client = new DBSQLClient();

0 commit comments

Comments
 (0)