Fix ~5x fetch regression by caching TelemetryCollector in DatabricksResultSet

[gopalldb]

Summary

• Cache the TelemetryCollector reference at DatabricksResultSet construction time instead of resolving it on every next() call
• Eliminates per-row overhead introduced by PR [PECOBLR-1408] [PECOBLR-1407] [PECOBLR-1409][PECOBLR-1411] Address telemetry code audit comments- part1 #1163 (telemetry refactor from singleton to per-connection)
• Restores Arrow cloud-fetch throughput to baseline levels

Problem

PR #1163 refactored telemetry collection from a singleton pattern to per-connection, but inadvertently introduced per-row overhead in DatabricksResultSet.next(). On every row iteration (~6.5M times for a 1025MB query), the code now traverses:

parentStatement.getStatement().getConnection()
  → cast to DatabricksConnection
  → getConnectionContext()
  → TelemetryCollectorManager.getInstance().getOrCreateCollector(context)
  → ConcurrentHashMap.computeIfAbsent()

This adds ~2.3 microseconds per row, compounding to ~15 seconds of overhead over 6.5M rows.

Fix

Resolve the TelemetryCollector once in each constructor via a resolveTelemetryCollector() helper and store it in a final field. The next() method calls the cached collector directly, bypassing the entire per-row resolution chain. The connection-to-collector mapping is stable for the lifetime of a result set, so caching at construction time is safe.

Before:

public boolean next() throws SQLException {
  checkIfClosed();
  boolean hasNext = this.executionResult.next();
  TelemetryHelper.recordResultSetIteration(
      parentStatement, statementId, resultSetMetaData.getChunkCount(), hasNext);
  return hasNext;
}

After:

public boolean next() throws SQLException {
  checkIfClosed();
  boolean hasNext = this.executionResult.next();
  if (cachedTelemetryCollector != null) {
    cachedTelemetryCollector.recordResultSetIteration(
        statementId.toSQLExecStatementId(), resultSetMetaData.getChunkCount(), hasNext);
  }
  return hasNext;
}

Benchmark Results

Regression Bisection (1025MB query, ~6.5M rows, Arrow cloud-fetch, Thrift mode, warm iteration)

Build	Commit	Fetch (ms)	Rows/s	Per-row cost	Status
v3.1.1 (baseline)	0e8b1ad	5,037	1.29M	0.78 us	✅ Baseline
Pre-#1163	9c9ffe2	4,720	1.38M	0.73 us	✅ Matches baseline
Post-#1163	8a2f333	19,727	329K	3.04 us	❌ 3.9x regression
v3.1.2 HEAD (unfixed)	47235a1	20,382	318K	3.14 us	❌ 4.0x regression
v3.1.2 (this fix)	eaa3f64	4,447	1.46M	0.68 us	✅ 12% faster than baseline

Post-Fix Segment Latency (500K rows per segment, warm iteration)

The fix restores the expected bursty pattern (fast in-memory iteration alternating with chunk-download waits):

 500K rows:   73ms    (in-memory, fast)
   1M rows:  393ms    (chunk wait)
 1.5M rows:  147ms    (in-memory)
   2M rows:  317ms    (chunk wait)
 2.5M rows:  409ms    (chunk wait)
   3M rows:  282ms    (mixed)
 3.5M rows:  250ms    (mixed)
   4M rows:  314ms    (chunk wait)
 4.5M rows:  197ms    (in-memory)
   5M rows:  370ms    (chunk wait)
 5.5M rows:  361ms    (chunk wait)
   6M rows:  146ms    (in-memory)

vs the unfixed v3.1.2 which showed flat ~1,450ms per segment (dominated by uniform per-row overhead).

Comprehensive Comparison (1025MB, warm iteration)

Driver	Mode	Arrow	Fetch (ms)	Rows/s
Simba 2.7.6	Thrift	N/A	109,994	59.0K
OSS v3.1.1	Thrift	Yes	5,037	1.29M
OSS v3.1.1	SEA	Yes	4,202	1.54M
OSS v3.1.1	Thrift	No	163,051	39.8K
OSS v3.1.2 (unfixed)	Thrift	Yes	20,382	318K
OSS v3.1.2 (unfixed)	SEA	Yes	19,882	326K
OSS v3.1.2 (unfixed)	Thrift	No	145,131	44.7K
OSS v3.1.2 (this fix)	Thrift	Yes	4,447	1.46M

NoArrow mode is unaffected by the regression (~145-163s for both versions) because the per-row telemetry overhead (~2.3us) is negligible compared to the ~25us/row cost of non-Arrow iteration.

Test plan

• mvn test -Dtest=DatabricksResultSetTest — all 48 tests pass (43 existing + 5 new)
• New tests cover: next() with cached telemetry, null parentStatement (no NPE), exception handling in resolution, null connection context, collector resolved once at construction
• Benchmark verified: fetch time drops from 20,382ms → 4,447ms (4.6x improvement)
• CI pipeline

🤖 Generated with Claude Code

NO_CHANGELOG=true
OVERRIDE_FREEZE=true

[samikshya-db]

as this was not released, do we need a changelog? wondering if that would confuse readers

[gopalldb]

fair point, will remove

[samikshya-db]

Thanks for fixing this!

[jayantsing-db]

why do we have to do this with every next? this is super wasteful and anti-pattern with no benefit. Please fix this to have it on fetch request boundary.

[vikrantpuppala]

+1, also why we needed #1210