refactor(bigquery-jdbc): migrate metadata thread management to connection-scoped executor

[keshavdandeva]

b/520400589

This PR optimizes thread management for JDBC metadata methods, implementing a deadlock-free, bipartite thread pool design and unifying resource lifecycle management.

Key Changes

• Bipartite Thread Pool Architecture (Deadlock Prevention):

  • Redirected all outer, orchestrating fetcher tasks (parent tasks) to the connection’s cached query executor (connection.getExecutorService()). Because the cached pool scales dynamically as needed, this structurally prevents pool-induced deadlocks (thread starvation).
  • Kept all inner, highly concurrent parallel network API calls (child tasks) on the connection's fixed-size metadata executor (connection.getMetadataExecutor()). This bounds parallel network traffic to prevent overwhelming the BigQuery backend.
  • Result: The driver is fully protected against pool-induced deadlocks, even under high concurrency or if metadataFetchThreadCount is configured to 1.

• Connection-Scoped Shared Executor:

  • Migrated all asynchronous metadata operations (getSchemas, getTables, getColumns, getProcedures, getProcedureColumns, getFunctions, and getFunctionColumns) from spawning raw threads or short-lived local pools to connection-managed executors.
  • Configured the shared metadata pool to be lazily instantiated on demand and gracefully shut down with the connection lifecycle. Idle threads are automatically reclaimed after 60 seconds.

• Lower CPU Overhead (Sequential Row-Building):

  • Completely removed secondary CPU-bound thread pools (such as routineProcessorExecutor, processArgsExecutor, tableProcessorExecutor, and processParamsExecutor) previously used for row-building.
  • Refactored row-building and parameter-processing tasks to execute sequentially on the background fetcher threads, removing context-switching overhead and preventing nested-pool deadlock risks.

• Teardown of Legacy Bridges:

  • Deleted the obsolete wrapThread(Thread) compatibility adapter in BigQueryDatabaseMetaData.java.
  • Removed obsolete testWrapThread_* cases from BigQueryDatabaseMetaDataTest.java. Metadata queries now track and cancel background tasks natively via standard Future<?> handles.

• Test Suite Refactoring:

  • Updated BigQueryDatabaseMetaDataTest setup to inject real, connection-scoped cached executor services, ensuring that background metadata tasks execute deterministically during testing.
  • Added @AfterEach teardown logic to prevent test-suite thread leaks.
  • Refactored argument-processing tests to verify row-building synchronously without mock executors.

[gemini-code-assist]

Code Review

This pull request refactors BigQueryDatabaseMetaData to use a shared, lazily-initialized metadataExecutor from BigQueryConnection instead of creating and shutting down local thread pools for metadata operations. Additionally, it simplifies argument and parameter processing by executing them sequentially rather than in parallel, and removes the temporary wrapThread utility. Feedback is provided regarding a potential NullPointerException during connection teardown, as the lazily-initialized metadataExecutor may remain null if no metadata queries are executed.

[keshavdandeva]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors metadata fetching in the BigQuery JDBC driver by lazily initializing the metadataExecutor in BigQueryConnection and utilizing it across various metadata operations in BigQueryDatabaseMetaData instead of spawning raw threads or local thread pools. Additionally, several processing tasks are refactored to run sequentially, and the temporary wrapThread utility is removed. The review comments identify two critical issues: first, a potential pool-induced deadlock (thread starvation) caused by submitting both parent fetcher tasks and child API tasks to the same limited fixed thread pool; second, a potential NullPointerException during connection closure because the lazily initialized metadataExecutor may remain null.

[keshavdandeva]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors metadata fetching in BigQueryDatabaseMetaData to utilize shared executors from BigQueryConnection instead of spawning local thread pools, and transitions several parallel tasks to sequential execution. The temporary wrapThread utility has also been removed. The review feedback points out critical concurrency and lifecycle issues: the lazy initialization of metadataExecutor in BigQueryConnection requires a null check in close() to prevent a NullPointerException and synchronization to avoid resource leaks. Furthermore, in BigQueryDatabaseMetaData, the reviewer suggests moving the cancellation of background futures to finally blocks to ensure they are always cancelled when the fetcher thread exits, while also properly restoring the thread's interrupted status.

[logachev]

Why changing this to lazy init? Init is cheap & it won't start threads unless required

[keshavdandeva]

I was thinking to optimize it for the cases when connections only execute SQL statements (and never run metadata queries) so it takes absolutely zero memory or object allocations for the metadata pool.

But yeah you are right, I missed Init is cheap & it won't start threads unless required. Reverted.

[keshavdandeva]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors BigQueryDatabaseMetaData to utilize the shared ExecutorService and metadataExecutor from BigQueryConnection instead of creating raw threads and ad-hoc thread pools. It also simplifies metadata processing by handling certain tasks sequentially and removes the temporary wrapThread compatibility bridge. Feedback suggests lazily initializing the metadataExecutor in BigQueryConnection to prevent unnecessary resource overhead and avoid potential initialization order issues.

[gemini-code-assist]

To avoid unnecessary performance and memory overhead when metadata methods are not called, initialize the metadataExecutor lazily inside getMetadataExecutor() instead of eagerly in the constructor. This also avoids any potential order-of-initialization issues with this.metadataFetchThreadCount during connection construction.

References

1. Prefer lazy initialization over eager initialization for resource-intensive objects if they are not guaranteed to be used in all execution paths, to avoid unnecessary performance and memory overhead.

[keshavdandeva]

No