Skip to content

Commit b227e3a

Browse files
rahuls-dbIsaac
andauthored
feat: auto-recover Reyden Thrift connections onto the kernel backend (#523)
* feat: auto-recover Reyden Thrift connections onto the kernel backend An unconfigured connection to a Reyden / Real-Time SQL warehouse defaults to the Thrift backend, which the SQL Gateway proxy rejects with SQLSTATE KP001. Detect that rejection (StatusError.sqlState === "KP001") in ThriftBackend.openSession and transparently re-open the session on the KernelBackend (SEA), remembering the warehouse in a process-wide cache keyed by (host, warehouse_id) with a ~6h TTL so later connects skip the doomed Thrift attempt. Only the default path auto-recovers; an explicit backend choice (routed upstream in the client) is unaffected. On a double failure the kernel error is surfaced with the original Thrift rejection preserved as its cause. Also re-throw the original error unchanged on the non-recovery paths: StatusError implements Error but does not extend it, so the previous `error instanceof Error ? error : new Error(String(error))` normalization wrapped every StatusError into Error("[object Object]"), losing its sqlState and the double-failure cause. Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com> * Address review: close fallback KernelBackend, test TTL expiry - ThriftBackend: track the KernelBackend(s) created for Reyden (KP001) fallback and close them in close(), so the process-global log-bridge onLevelChange listener installed by connect() is released instead of leaking on every recovery. Add a createKernelBackend() seam so tests can inject a fake without the native binding, plus a test that close() releases the fallback backend. - ReydenWarehouseCache test: add a TTL-expiry test (sinon fake timers) covering the 6h boundary and opportunistic eviction on access. Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com> * Address review: reuse one fallback KernelBackend, guard cause overwrite - ThriftBackend: reuse a single fallback KernelBackend across all Reyden (KP001) fallback sessions on the connection instead of constructing one per openSession. connectionOptions are fixed after connect, so it is created + connected once (lazily, memoized; the attempt is cleared on connect failure so a later open can retry) and released in close() — this stops per-session accumulation of backends and process-global log-bridge listeners. - On the double-failure path, only set the kernel error's `cause` when it is absent, so a cause the kernel error may already carry is not clobbered. - Test now opens two fallback sessions and asserts a single KernelBackend is created and connected once, reused for both, and closed once on close(). Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com> * Sweep expired entries from the Reyden warehouse cache on mark markReyden only added the new entry, so the per-key lazy eviction in isKnownReyden never reclaimed an entry that was marked and then never looked up again — it would persist for the life of the process. Sweep expired entries when marking a warehouse; markReyden runs only on an actual Thrift rejection, so the sweep is near-free and bounds the cache to warehouses seen within the TTL window. Adds a fake-timer test. Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com> * Close a fallback KernelBackend whose connect races close() close() released the fallback backend only via this.fallbackKernelBackend, which getFallbackKernelBackend assigns after connect() resolves. A close() that ran while a fallback connect was still in flight found the field unset and skipped it; the pending connect then resolved, installed the process- global log-bridge listener, and assigned the field on a backend nobody would ever close — leaking that listener for the process lifetime. Await the in-flight connect promise in close() instead of only the resolved field, closing whatever it produces (whether already connected or still in flight). Adds a test that calls close() during an unresolved fallback connect. Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com> * Drop the write-only fallbackKernelBackend field After close() switched to awaiting the in-flight connect, the fallbackKernelBackend field became write-only: assigned in the connect IIFE and cleared in close(), but read nowhere. It also left a confusing stale write — a close() racing an in-flight connect nulled the field before the IIFE re-assigned it onto an already-closed backend. Remove the field; the fallbackKernelBackendConnect promise (whose resolved value is the backend) is the single source of truth. Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com> * Make the Reyden cache presence-based, dropping the dead tri-state markReyden was the only writer and only ever stored isReyden: true, so the CacheEntry.isReyden field and isKnownReyden's boolean | undefined return type carried an unreachable "known not Reyden" state. Drop the field and return a plain boolean: an unexpired entry means Reyden, its absence means not known. Callers already used the result in a boolean context. Update the tests that asserted the old undefined return to expect false. Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com> --------- Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com> Co-authored-by: Isaac <no-reply@databricks.com>
1 parent 28694bc commit b227e3a

5 files changed

Lines changed: 631 additions & 3 deletions

File tree

lib/ReydenWarehouseCache.ts

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/**
2+
* Process-wide cache for tracking Reyden (Real-Time SQL) warehouses.
3+
*
4+
* When a Thrift OpenSession fails with SQLSTATE KP001, the driver falls back
5+
* to the SEA (Statement Execution API) backend. This cache avoids retrying
6+
* the same failed Thrift path on subsequent connections by recording which
7+
* warehouses are known to require SEA.
8+
*
9+
* The cache is keyed by (host_lowercased, warehouse_id) to handle multi-tenant
10+
* safety — the same warehouse ID on different hosts may have different support.
11+
*
12+
* TTL is ~6 hours to allow the server side to update warehouse routing without
13+
* requiring a process restart. Expired entries are opportunistically evicted on
14+
* access (no background GC thread — Node is single-threaded).
15+
*/
16+
17+
const TTL_MS = 6 * 60 * 60 * 1000; // 6 hours
18+
19+
interface CacheEntry {
20+
timestamp: number;
21+
}
22+
23+
class ReydenWarehouseCache {
24+
private static instance?: ReydenWarehouseCache;
25+
26+
private cache: Map<string, CacheEntry> = new Map();
27+
28+
// Singleton: constructor is private to enforce getInstance() usage
29+
// eslint-disable-next-line @typescript-eslint/no-empty-function
30+
private constructor() {}
31+
32+
public static getInstance(): ReydenWarehouseCache {
33+
if (!ReydenWarehouseCache.instance) {
34+
ReydenWarehouseCache.instance = new ReydenWarehouseCache();
35+
}
36+
return ReydenWarehouseCache.instance;
37+
}
38+
39+
/**
40+
* Constructs a cache key from host and warehouse ID.
41+
* Host is lowercased for case-insensitive comparison.
42+
*/
43+
private getKey(host: string, warehouseId: string): string {
44+
return `${host.toLowerCase()}:${warehouseId}`;
45+
}
46+
47+
/**
48+
* Check if an entry is expired based on TTL.
49+
*/
50+
private isExpired(entry: CacheEntry): boolean {
51+
return Date.now() - entry.timestamp > TTL_MS;
52+
}
53+
54+
/**
55+
* Checks if a warehouse is known to be Reyden (requiring SEA fallback).
56+
*
57+
* Membership is presence-based: the cache only ever records known-Reyden
58+
* warehouses (via markReyden), so an unexpired entry means Reyden and the
59+
* absence of one means "not known" — there is no negative-cache state.
60+
* Returns false when the warehouse is not in the cache or the entry expired.
61+
*/
62+
public isKnownReyden(host: string, warehouseId: string): boolean {
63+
const key = this.getKey(host, warehouseId);
64+
const entry = this.cache.get(key);
65+
66+
if (!entry) {
67+
return false;
68+
}
69+
70+
// Opportunistically evict expired entries on access
71+
if (this.isExpired(entry)) {
72+
this.cache.delete(key);
73+
return false;
74+
}
75+
76+
return true;
77+
}
78+
79+
/**
80+
* Mark a warehouse as being Reyden (KP001 rejection detected).
81+
*/
82+
public markReyden(host: string, warehouseId: string): void {
83+
const now = Date.now();
84+
85+
// Opportunistic sweep: markReyden runs only on an actual Thrift rejection
86+
// (rare), so purging every expired entry here is near-free and bounds the
87+
// cache to warehouses seen within the TTL window. The per-key lazy eviction
88+
// in isKnownReyden only reclaims entries that are looked up again, so an
89+
// entry that is never queried after marking would otherwise persist for the
90+
// life of the process.
91+
for (const [existingKey, entry] of this.cache) {
92+
if (now - entry.timestamp > TTL_MS) {
93+
this.cache.delete(existingKey);
94+
}
95+
}
96+
97+
this.cache.set(this.getKey(host, warehouseId), { timestamp: now });
98+
}
99+
100+
/**
101+
* Clears the cache. Intended for testing only.
102+
*
103+
* @internal
104+
*/
105+
public clear(): void {
106+
this.cache.clear();
107+
}
108+
109+
/**
110+
* Returns the current cache size. Intended for testing/observability.
111+
*
112+
* @internal
113+
*/
114+
public size(): number {
115+
return this.cache.size;
116+
}
117+
}
118+
119+
export default ReydenWarehouseCache.getInstance();

lib/errors/StatusError.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,15 @@ export default class StatusError implements Error {
77

88
public code: number;
99

10+
public sqlState?: string;
11+
1012
public stack?: string;
1113

1214
constructor(status: TStatus) {
1315
this.name = 'Status Error';
1416
this.message = status.errorMessage || '';
1517
this.code = status.errorCode || -1;
18+
this.sqlState = status.sqlState;
1619

1720
if (Array.isArray(status.infoMessages)) {
1821
this.stack = status.infoMessages.join('\n');

lib/thrift-backend/ThriftBackend.ts

Lines changed: 146 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,15 @@ import Int64 from 'node-int64';
22
import IBackend from '../contracts/IBackend';
33
import ISessionBackend from '../contracts/ISessionBackend';
44
import IClientContext from '../contracts/IClientContext';
5-
import { OpenSessionRequest } from '../contracts/IDBSQLClient';
5+
import { ConnectionOptions, OpenSessionRequest } from '../contracts/IDBSQLClient';
66
import { TProtocolVersion } from '../../thrift/TCLIService_types';
77
import Status from '../dto/Status';
88
import { definedOrError, serializeQueryTags } from '../utils';
99
import ThriftSessionBackend from './ThriftSessionBackend';
10+
import StatusError from '../errors/StatusError';
11+
import reydenCache from '../ReydenWarehouseCache';
12+
import KernelBackend from '../kernel/KernelBackend';
13+
import { LogLevel } from '../contracts/IDBSQLLogger';
1014

1115
function getInitialNamespaceOptions(catalogName?: string, schemaName?: string) {
1216
if (!catalogName && !schemaName) {
@@ -31,12 +35,44 @@ export default class ThriftBackend implements IBackend {
3135

3236
private readonly onConnectionEvent: ThriftBackendOptions['onConnectionEvent'];
3337

38+
private connectionOptions?: ConnectionOptions;
39+
40+
// The memoized connect for a single KernelBackend, reused across every Reyden (KP001)
41+
// fallback session on this connection. connect() installs a process-global log-bridge
42+
// listener, so the backend is created once (connectionOptions are fixed after connect)
43+
// and released in close() — rather than constructing one per openSession and leaking a
44+
// listener each time. This promise is the single source of truth for the fallback
45+
// backend; its resolved value is the backend.
46+
private fallbackKernelBackendConnect?: Promise<KernelBackend>;
47+
3448
constructor({ context, onConnectionEvent }: ThriftBackendOptions) {
3549
this.context = context;
3650
this.onConnectionEvent = onConnectionEvent;
3751
}
3852

39-
public async connect(): Promise<void> {
53+
/**
54+
* Extracts warehouse/endpoint ID from the HTTP path.
55+
* Matches patterns like `/sql/1.0/warehouses/<id>` or `/sql/1.0/endpoints/<id>`.
56+
* Returns undefined if no ID can be extracted.
57+
*/
58+
private static extractWarehouseId(httpPath: string | undefined): string | undefined {
59+
if (!httpPath) {
60+
return undefined;
61+
}
62+
63+
// Stop at query string
64+
const pathOnly = httpPath.split('?')[0];
65+
66+
// Match `/warehouses/<id>` or `/endpoints/<id>`
67+
// Stop at `/` or end of string
68+
const match = pathOnly.match(/\/(warehouses|endpoints)\/([^/]+)/);
69+
return match ? match[2] : undefined;
70+
}
71+
72+
public async connect(options: ConnectionOptions): Promise<void> {
73+
// Store connection options for warehouse ID extraction in openSession
74+
this.connectionOptions = options;
75+
4076
// The connection provider is owned by DBSQLClient (it implements IClientContext).
4177
// We only need to wire the EventEmitter listeners through this backend.
4278
const connectionProvider = await this.context.getConnectionProvider();
@@ -60,6 +96,63 @@ export default class ThriftBackend implements IBackend {
6096
}
6197

6298
public async openSession(request: OpenSessionRequest): Promise<ISessionBackend> {
99+
const logger = this.context.getLogger();
100+
101+
// Extract warehouse ID for cache lookups
102+
const warehouseId = ThriftBackend.extractWarehouseId(this.connectionOptions?.path);
103+
const host = this.connectionOptions?.host;
104+
105+
// Check if this warehouse is known to be Reyden (requires SEA backend)
106+
if (host && warehouseId && reydenCache.isKnownReyden(host, warehouseId)) {
107+
logger.log(LogLevel.debug, `Reyden: warehouse ${warehouseId} is known to require SEA fallback; skipping Thrift`);
108+
return this.openSessionWithKernelBackend(request);
109+
}
110+
111+
// Try Thrift first (default path).
112+
try {
113+
return await this.openSessionWithThrift(request);
114+
} catch (error) {
115+
// Only a Reyden KP001 rejection triggers fallback. Every other error
116+
// propagates unchanged — note StatusError is NOT an Error subclass
117+
// (it only `implements Error`), so it must be re-thrown as-is rather
118+
// than normalized, or its sqlState/message would be lost.
119+
if (error instanceof StatusError && error.sqlState === 'KP001') {
120+
logger.log(LogLevel.debug, `Reyden: detected KP001 on warehouse ${warehouseId}; falling back to SEA backend`);
121+
122+
// Mark this warehouse as Reyden for future connections.
123+
if (host && warehouseId) {
124+
reydenCache.markReyden(host, warehouseId);
125+
}
126+
127+
// Fall back to the kernel (SEA) backend exactly once. If it also fails,
128+
// surface the kernel error but keep the original Thrift rejection as its
129+
// cause for diagnosis.
130+
try {
131+
return await this.openSessionWithKernelBackend(request);
132+
} catch (kernelError) {
133+
// Preserve the Thrift KP001 as the kernel error's cause, but don't clobber a cause
134+
// the kernel error may already carry.
135+
if (
136+
kernelError &&
137+
typeof kernelError === 'object' &&
138+
(kernelError as { cause?: unknown }).cause === undefined
139+
) {
140+
(kernelError as { cause?: unknown }).cause = error;
141+
}
142+
logger.log(LogLevel.error, 'Reyden: both Thrift (KP001) and SEA fallback failed');
143+
throw kernelError;
144+
}
145+
}
146+
147+
// Not a Reyden rejection — surface the original error unchanged.
148+
throw error;
149+
}
150+
}
151+
152+
/**
153+
* Opens a session using the Thrift backend.
154+
*/
155+
private async openSessionWithThrift(request: OpenSessionRequest): Promise<ISessionBackend> {
63156
const driver = await this.context.getDriver();
64157
const config = this.context.getConfig();
65158

@@ -93,8 +186,58 @@ export default class ThriftBackend implements IBackend {
93186
});
94187
}
95188

189+
/**
190+
* Opens a session using the KernelBackend (SEA).
191+
* Called as a fallback when Thrift returns KP001 (Reyden rejection).
192+
*/
193+
private async openSessionWithKernelBackend(request: OpenSessionRequest): Promise<ISessionBackend> {
194+
if (!this.connectionOptions) {
195+
throw new Error('KernelBackend fallback: connection options not available');
196+
}
197+
198+
const logger = this.context.getLogger();
199+
logger.log(LogLevel.debug, 'Reyden: opening session via KernelBackend (SEA)');
200+
201+
const kernelBackend = await this.getFallbackKernelBackend(this.connectionOptions);
202+
return kernelBackend.openSession(request);
203+
}
204+
205+
// Lazily creates and connects the single fallback KernelBackend, reused across every
206+
// fallback session so repeated opens don't accumulate backends / log-bridge listeners.
207+
// On a connect failure the memoized attempt is cleared so a later open can retry.
208+
private getFallbackKernelBackend(connectionOptions: ConnectionOptions): Promise<KernelBackend> {
209+
if (!this.fallbackKernelBackendConnect) {
210+
this.fallbackKernelBackendConnect = (async () => {
211+
const kernelBackend = this.createKernelBackend();
212+
await kernelBackend.connect(connectionOptions);
213+
return kernelBackend;
214+
})().catch((error) => {
215+
this.fallbackKernelBackendConnect = undefined;
216+
throw error;
217+
});
218+
}
219+
return this.fallbackKernelBackendConnect;
220+
}
221+
222+
// Seam so tests can inject a fake KernelBackend without the native binding.
223+
protected createKernelBackend(): KernelBackend {
224+
return new KernelBackend({ context: this.context });
225+
}
226+
96227
public async close(): Promise<void> {
97-
// DBSQLClient owns the connection lifecycle and clears its own state
228+
// Release the process-global log-bridge listener held by the Reyden-fallback KernelBackend.
229+
// DBSQLClient owns the rest of the connection lifecycle and clears its own state
98230
// (connectionProvider, authProvider, thrift client) after this returns.
231+
//
232+
// Await the in-flight connect rather than a resolved-backend field: the connect
233+
// installs the listener only once it resolves, so a close() racing an unresolved
234+
// fallback connect must still wait for it and release the backend it produces.
235+
// Clear the field first so the state is consistent even if the awaited close() throws.
236+
const pendingConnect = this.fallbackKernelBackendConnect;
237+
this.fallbackKernelBackendConnect = undefined;
238+
if (pendingConnect) {
239+
const kernelBackend = await pendingConnect.catch(() => undefined);
240+
await kernelBackend?.close();
241+
}
99242
}
100243
}

0 commit comments

Comments
 (0)