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>

Author: rahuls-db

lib/thrift-backend/ThriftBackend.ts

@@ -37,10 +37,13 @@ export default class ThriftBackend implements IBackend {
 
   private connectionOptions?: ConnectionOptions;
 
-  // KernelBackend(s) created for Reyden (KP001) fallback. Tracked so their
-  // process-global log-bridge listeners are released on close() — otherwise each
-  // fallback session would leak an onLevelChange listener for the process lifetime.
-  private fallbackKernelBackends: KernelBackend[] = [];
+  // A single KernelBackend reused for every Reyden (KP001) fallback session on this
+  // connection. connect() installs a process-global log-bridge listener, so it is created
+  // once (connectionOptions are fixed after connect) and released in close() — rather than
+  // constructing one per openSession and leaking a listener each time.
+  private fallbackKernelBackend?: KernelBackend;
+
+  private fallbackKernelBackendConnect?: Promise<KernelBackend>;
 
   constructor({ context, onConnectionEvent }: ThriftBackendOptions) {
     this.context = context;
@@ -127,7 +130,13 @@ export default class ThriftBackend implements IBackend {
         try {
           return await this.openSessionWithKernelBackend(request);
         } catch (kernelError) {
-          if (kernelError && typeof kernelError === 'object') {
+          // Preserve the Thrift KP001 as the kernel error's cause, but don't clobber a cause
+          // the kernel error may already carry.
+          if (
+            kernelError &&
+            typeof kernelError === 'object' &&
+            (kernelError as { cause?: unknown }).cause === undefined
+          ) {
             (kernelError as { cause?: unknown }).cause = error;
           }
           logger.log(LogLevel.error, 'Reyden: both Thrift (KP001) and SEA fallback failed');
@@ -189,24 +198,41 @@ export default class ThriftBackend implements IBackend {
     const logger = this.context.getLogger();
     logger.log(LogLevel.debug, 'Reyden: opening session via KernelBackend (SEA)');
 
-    // Create a KernelBackend and connect/open. Track it so close() releases the
-    // log-bridge listener that connect() installs.
-    const kernelBackend = this.createKernelBackend();
-    this.fallbackKernelBackends.push(kernelBackend);
-    await kernelBackend.connect(this.connectionOptions);
+    const kernelBackend = await this.getFallbackKernelBackend(this.connectionOptions);
     return kernelBackend.openSession(request);
   }
 
+  // Lazily creates and connects the single fallback KernelBackend, reused across every
+  // fallback session so repeated opens don't accumulate backends / log-bridge listeners.
+  // On a connect failure the memoized attempt is cleared so a later open can retry.
+  private getFallbackKernelBackend(connectionOptions: ConnectionOptions): Promise<KernelBackend> {
+    if (!this.fallbackKernelBackendConnect) {
+      this.fallbackKernelBackendConnect = (async () => {
+        const kernelBackend = this.createKernelBackend();
+        await kernelBackend.connect(connectionOptions);
+        this.fallbackKernelBackend = kernelBackend;
+        return kernelBackend;
+      })().catch((error) => {
+        this.fallbackKernelBackendConnect = undefined;
+        throw error;
+      });
+    }
+    return this.fallbackKernelBackendConnect;
+  }
+
   // Seam so tests can inject a fake KernelBackend without the native binding.
   protected createKernelBackend(): KernelBackend {
     return new KernelBackend({ context: this.context });
   }
 
   public async close(): Promise<void> {
-    // Release the process-global log-bridge listener(s) held by any Reyden-fallback
-    // KernelBackend. DBSQLClient owns the rest of the connection lifecycle and clears
-    // its own state (connectionProvider, authProvider, thrift client) after this returns.
-    await Promise.all(this.fallbackKernelBackends.map((backend) => backend.close()));
-    this.fallbackKernelBackends = [];
+    // Release the process-global log-bridge listener held by the Reyden-fallback KernelBackend.
+    // DBSQLClient owns the rest of the connection lifecycle and clears its own state
+    // (connectionProvider, authProvider, thrift client) after this returns.
+    if (this.fallbackKernelBackend) {
+      await this.fallbackKernelBackend.close();
+      this.fallbackKernelBackend = undefined;
+      this.fallbackKernelBackendConnect = undefined;
+    }
   }
 }

tests/unit/thrift-backend/ReydenThriftRecoveryOrchestration.test.ts

@@ -126,7 +126,7 @@ describe('Reyden Thrift Auto-Recovery — Orchestration', () => {
     expect(thrown.cause).to.equal(thriftError);
   });
 
-  it('closes the fallback KernelBackend on close(), releasing its log-bridge listener', async () => {
+  it('reuses one fallback KernelBackend across sessions and closes it on close()', async () => {
     const backend = makeBackend();
     const fakeKernel = {
       connect: sandbox.stub().resolves(),
@@ -135,14 +135,19 @@ describe('Reyden Thrift Auto-Recovery — Orchestration', () => {
     };
     sandbox.stub(backend as any, 'openSessionWithThrift').rejects(kp001Error());
     // Inject the fake via the createKernelBackend seam and let the REAL
-    // openSessionWithKernelBackend run (connect + track), so close() must release it.
-    sandbox.stub(backend as any, 'createKernelBackend').returns(fakeKernel as any);
+    // openSessionWithKernelBackend run (connect + reuse), so close() must release it.
+    const createStub = sandbox.stub(backend as any, 'createKernelBackend').returns(fakeKernel as any);
 
-    await backend.openSession({} as any);
+    await backend.openSession({} as any); // reactive recovery marks the cache
+    await backend.openSession({} as any); // second open hits the pre-check → same fallback backend
+
+    // A single KernelBackend is created and connected once, not one per session.
+    expect(createStub.calledOnce).to.be.true;
     expect(fakeKernel.connect.calledOnce).to.be.true;
-    expect(fakeKernel.close.called).to.be.false; // still open
+    expect(fakeKernel.openSession.calledTwice).to.be.true;
+    expect(fakeKernel.close.called).to.be.false; // not closed until close()
 
     await backend.close();
-    expect(fakeKernel.close.calledOnce).to.be.true; // released on ThriftBackend.close()
+    expect(fakeKernel.close.calledOnce).to.be.true; // released once on ThriftBackend.close()
   });
 });