From c5a50fc3a8e8a781ec431ebdf687c5a5965584d9 Mon Sep 17 00:00:00 2001 From: Madhavendra Rathore Date: Wed, 10 Jun 2026 21:51:55 +0000 Subject: [PATCH] feat: make retry-policy knobs configurable via connect() (both backends) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 --- lib/DBSQLClient.ts | 18 +++++++++++++++++ lib/contracts/IDBSQLClient.ts | 23 ++++++++++++++++++++++ tests/unit/DBSQLClient.test.ts | 36 ++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+) diff --git a/lib/DBSQLClient.ts b/lib/DBSQLClient.ts index 108e220f..8054c7c8 100644 --- a/lib/DBSQLClient.ts +++ b/lib/DBSQLClient.ts @@ -611,6 +611,24 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I this.config.preserveBigNumericPrecision = options.preserveBigNumericPrecision; } + // Retry-policy knobs. These live in ClientConfig (consumed by the Thrift + // HttpRetryPolicy and forwarded to the kernel on the SEA path), so copying + // them here makes them configurable from connect() on BOTH backends. A + // per-key narrowed copy (not a spread) keeps the structural type system + // honest, matching the metricView / precision / telemetry knobs above. + if (options.retryMaxAttempts !== undefined) { + this.config.retryMaxAttempts = options.retryMaxAttempts; + } + if (options.retriesTimeout !== undefined) { + this.config.retriesTimeout = options.retriesTimeout; + } + if (options.retryDelayMin !== undefined) { + this.config.retryDelayMin = options.retryDelayMin; + } + if (options.retryDelayMax !== undefined) { + this.config.retryDelayMax = options.retryDelayMax; + } + // Override telemetry config if provided in options. Per-key narrowed copy // preserves the structural type system: `ConnectionOptions` and // `ClientConfig` declare identical types for these knobs, so a user diff --git a/lib/contracts/IDBSQLClient.ts b/lib/contracts/IDBSQLClient.ts index de67070e..b1b2bb7b 100644 --- a/lib/contracts/IDBSQLClient.ts +++ b/lib/contracts/IDBSQLClient.ts @@ -59,6 +59,29 @@ export type ConnectionOptions = { proxy?: ProxyOptions; enableMetricViewMetadata?: boolean; + /** + * Retry-policy knobs governing how the driver retries retryable requests. + * They apply to **both** backends: the Thrift `HttpRetryPolicy` reads them + * directly, and on the kernel (SEA) path they are forwarded to the kernel + * (which owns the retry loop) via `buildKernelRetryOptions`. An unset field + * keeps the driver default shown below. + * + * • `retryMaxAttempts` — maximum TOTAL number of attempts (the initial + * request plus any retries). Default 5. `0` or `1` both mean a single + * attempt with no retry. Both backends honour the same total-attempt + * semantics (the kernel converts it to its after-initial retry count). + * • `retriesTimeout` — maximum total wallclock spent retrying, in + * milliseconds. Default 900000 (15 minutes). + * • `retryDelayMin` — minimum backoff between attempts, in milliseconds. + * Default 1000. + * • `retryDelayMax` — maximum backoff between attempts, in milliseconds. + * Default 60000. + */ + retryMaxAttempts?: number; + retriesTimeout?: number; + retryDelayMin?: number; + retryDelayMax?: number; + /** * Preserve full numeric precision in results. When `true`, DECIMAL columns * are returned as exact strings and 64-bit integers (BIGINT) as JS `bigint`, diff --git a/tests/unit/DBSQLClient.test.ts b/tests/unit/DBSQLClient.test.ts index ab45251b..4cf16838 100644 --- a/tests/unit/DBSQLClient.test.ts +++ b/tests/unit/DBSQLClient.test.ts @@ -620,6 +620,42 @@ describe('DBSQLClient.createAuthProvider', () => { }); }); +describe('DBSQLClient retry-policy ConnectionOptions', () => { + it('ingests retry-policy options from connect() into ClientConfig', async () => { + const client = new DBSQLClient(); + + // Defaults before connect. + expect(client.getConfig().retryMaxAttempts).to.equal(5); + expect(client.getConfig().retriesTimeout).to.equal(15 * 60 * 1000); + expect(client.getConfig().retryDelayMin).to.equal(1000); + expect(client.getConfig().retryDelayMax).to.equal(60 * 1000); + + await client.connect({ + ...connectOptions, + retryMaxAttempts: 2, + retriesTimeout: 5000, + retryDelayMin: 100, + retryDelayMax: 500, + }); + + expect(client.getConfig().retryMaxAttempts).to.equal(2); + expect(client.getConfig().retriesTimeout).to.equal(5000); + expect(client.getConfig().retryDelayMin).to.equal(100); + expect(client.getConfig().retryDelayMax).to.equal(500); + }); + + it('keeps defaults when retry-policy options are omitted', async () => { + const client = new DBSQLClient(); + + await client.connect({ ...connectOptions }); + + expect(client.getConfig().retryMaxAttempts).to.equal(5); + expect(client.getConfig().retriesTimeout).to.equal(15 * 60 * 1000); + expect(client.getConfig().retryDelayMin).to.equal(1000); + expect(client.getConfig().retryDelayMax).to.equal(60 * 1000); + }); +}); + describe('DBSQLClient.enableMetricViewMetadata', () => { it('should store enableMetricViewMetadata config when enabled', async () => { const client = new DBSQLClient();