Skip to content

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

Open
keshavdandeva wants to merge 3 commits into
mainfrom
jdbc/metadata-thread-refactor
Open

refactor(bigquery-jdbc): migrate metadata thread management to connection-scoped executor#13556
keshavdandeva wants to merge 3 commits into
mainfrom
jdbc/metadata-thread-refactor

Conversation

@keshavdandeva

@keshavdandeva keshavdandeva commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

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 gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@keshavdandeva keshavdandeva marked this pull request as ready for review June 25, 2026 15:00
@keshavdandeva keshavdandeva requested review from a team as code owners June 25, 2026 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants