databricks-sql-nodejs icon indicating copy to clipboard operation
databricks-sql-nodejs copied to clipboard

Fix flaky Iterator tests by making metadata fetching async-safe

Open shivam2680 opened this issue 3 months ago • 0 comments

Problem

The flaky Iterator tests were failing with: java.lang.IllegalStateException: Should not get result schema on an empty result handler

This occurred due to two related race conditions:

Root Cause 1: Missing Operation Readiness Check

The hasMoreRows() method was attempting to fetch result metadata before the operation finished executing. When tests used maxRows: null (disabling direct results), the operation had no cached metadata and would try to fetch it while the server-side operation was still running, causing the result handler to be in an invalid state.

Unlike other methods (getSchema(), fetchChunk()), hasMoreRows() did NOT call waitUntilReady() before accessing metadata.

Root Cause 2: Concurrent Metadata Fetches

When multiple concurrent calls to fetchMetadata() occurred before the first completed:

  • Each would check if (!this.metadata) and find it undefined
  • Multiple requests would be sent to the server simultaneously
  • This could result in fetching metadata after the result handler was consumed

Fix

  1. Wait for Operation Readiness in hasMoreRows()

Added waitUntilReady() call to ensure the operation completes before fetching metadata:

public async hasMoreRows(): Promise { if (this.closed || this.cancelled) { return false; }

  // Wait for operation to finish before checking for more rows
  if (this.operationHandle.hasResultSet) {
    await this.waitUntilReady();
  }

  const resultHandler = await this.getResultHandler();
  return resultHandler.hasMore();

}

  1. Async-Safe Metadata Fetching

Implemented promise-based locking using metadataPromise field:

  • First concurrent call creates and caches the promise
  • Subsequent concurrent calls wait for the same promise instead of creating new requests
  • Promise is cleared after completion (success or failure)

Tests Added

  1. Test for concurrent metadata fetch requests (verifies only 1 server call)
  2. Test for metadata caching after first fetch

shivam2680 avatar Oct 15 '25 19:10 shivam2680