fix(bigtable): bound SessionPoolImpl lock to prevent pod-wide wedge - #13890
Merged
Conversation
Pods running the vRPC session pool intermittently went permanently wedged
(recurring ~7-9h): in-flight requests piled up, RED fired, and the pod shed
100% of traffic with no recovery until restart.
Root cause (from a production thread dump, 2026-07-23):
- SessionPoolImpl guarded all of its state with a single `synchronized(this)`
intrinsic monitor.
- The dump showed that monitor with NO owner but 39 waiters
(22 onVRpcComplete, 8 onSessionGoAway, 6 onSessionReady, plus watchdog /
cancel / deadline threads) — the classic signature of an *orphaned monitor*:
a thread died while holding the lock (e.g. a native-thread OOM,
"unable to create native thread", mid-critical-section) so the monitor was
never released.
- The shim probes the pool on EVERY rpc via hasSession()/newCall(), both
synchronized. Once the monitor was orphaned, every incoming request blocked
on it forever. An intrinsic monitor has no timeout, so this was unrecoverable.
Note on the native-thread OOM: it is transient/recoverable on its own (unlike a
heap OOM). The damage that persisted was the orphaned lock it left behind — that
is what turned a momentary resource blip into a permanent, pod-wide wedge.
Fix: replace the intrinsic monitor with an explicit ReentrantLock (`poolLock`)
and use a bounded tryLock on the request-serving hot path
(HOT_PATH_LOCK_TIMEOUT_MILLIS = 1000). If the lock can't be acquired in time the
caller degrades instead of blocking forever:
- newCall() -> returns a PendingVRpc (start() re-attempts, then fast-fails)
- hasSession() -> returns false
- PendingVRpc.start() -> fast-fails the vRPC with an uncommitted UNAVAILABLE,
so gax can still retry / fall back to the classic client
- PendingVRpc.cancel() -> also uses the bounded acquisition. cancel() runs on a
callback (op-executor) thread; on the old code its unbounded lock() would
park that thread on the wedged lock — the exact pile-up seen in the dump.
The eager pendingRpcs.remove() is only an optimization; drainTo()'s
isCancelled guard is the correctness backstop, so skipping it on timeout is
safe.
This converts a permanent hang into a bounded, retriable fast-fail, so the pod
sheds a wedged pool's traffic transiently and recovers automatically once the
lock frees.
tryAcquireHotPathLock() first does a non-interruptible zero-arg tryLock() before
the timed wait: the timed tryLock(timeout) throws InterruptedException
immediately if the caller already has its interrupt flag set (even when the lock
is free), which the old `synchronized` never observed. Without the fast path a
pre-existing interrupt on a reused worker thread would spuriously shed traffic on
a healthy pool.
Watchdog: its lock field changes Object -> Lock and is now constructed with the
pool's poolLock; forceClose runs outside the lock (snapshot-then-release), and it
never nests poolLock with scheduleLock.
Tests:
- orphanedPoolLock_hotPathDegradesInsteadOfHangingForever: simulates the dead
owner (a thread that grabs poolLock and never releases) and asserts
hasSession/newCall/start/cancel all degrade bounded, then the pool recovers
once the lock frees. Fails by timeout on the old synchronized design.
- healthyPool_preExistingInterrupt_doesNotSpuriouslyDegrade: guards the
zero-arg fast path — an already-interrupted caller must still be served.
- WatchdogTest updated for the Lock-typed constructor.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request replaces the intrinsic synchronized monitor in SessionPoolImpl with an explicit ReentrantLock to prevent threads from hanging indefinitely if the lock becomes wedged or orphaned. Bounded lock acquisition is introduced on the request-serving hot path, allowing the pool to degrade gracefully and fast-fail instead of blocking forever. Additionally, comprehensive unit tests are added to verify the behavior under wedged lock conditions and to ensure that pre-existing thread interrupts do not spuriously degrade a healthy pool. As there are no review comments, no further feedback is provided.
mutianf
enabled auto-merge (squash)
July 24, 2026 18:53
whowes
pushed a commit
that referenced
this pull request
Jul 30, 2026
🤖 I have created a release *beep* *boop* --- <details><summary>1.89.0</summary> ## [1.89.0](v1.88.0...v1.89.0) (2026-07-29) ### Features * **agentidentity:** onboard v1 and v1beta API versions ([#13796](#13796)) ([1b1300d](1b1300d)) * **auth:** add JSpecify Null annotations to Auth ([#13842](#13842)) ([f6f24d4](f6f24d4)) * **bigquery-jdbc:** add `SSLTrustStoreType` and `SSLTrustStoreProvider` connection properties ([#13858](#13858)) ([9449be1](9449be1)) * **bigquery-jdbc:** add otel trace and span IDs to local logs ([#13935](#13935)) ([2801fbd](2801fbd)) * **bigquery-jdbc:** implement BigQueryParameterMetaData and dynamic type mappings ([#13812](#13812)) ([3d67dba](3d67dba)) * **bigquery-jdbc:** implement parameter setters in PreparedStatement ([#13792](#13792)) ([94f7404](94f7404)) * **bigquery-jdbc:** Migrate `getImportedKeys` and `getCrossReference` to BQ API ([#13692](#13692)) ([082b046](082b046)) * **bigquery-jdbc:** migrate `getPrimaryKeys` to use BQ API ([#13691](#13691)) ([1951f49](1951f49)) * **bigquery-jdbc:** OpenTelemetry integration in BQ JDBC ([#12902](#12902)) ([af18f65](af18f65)) * **bigquery-jdbc:** optimize memory footprint for JSON result set streaming ([#13660](#13660)) ([11f26d3](11f26d3)) * **bigquery-jdbc:** standardize parameter handling and calendar defensive copying across statement interfaces ([#13805](#13805)) ([ccd13eb](ccd13eb)) * **bigtable:** add view_parameters support to BoundStatement ([#13673](#13673)) ([d5cc437](d5cc437)) * **bigtable:** BigtableDataClientFactory session support ([#13829](#13829)) ([284ce13](284ce13)) * **commerceproducer:** onboard v1beta API ([#13814](#13814)) ([61b89b3](61b89b3)) * Default to least-in-flight balancing for Bigtable unary clients ([#13802](#13802)) ([ac9ccd1](ac9ccd1)) * **firestore:** Add support for 16MB documents ([#13478](#13478)) ([1b7c2e0](1b7c2e0)) * **gapic-generator:** add JSpecify Null annotations to the generator classes ([#13769](#13769)) ([843bd7c](843bd7c)) * **gapic-generator:** Add Nullable annotation to generated classes ([#13558](#13558)) ([e3e9d0b](e3e9d0b)) * **gapic-generator:** Add NullMarked annotation to generated classes ([#13584](#13584)) ([b7a8504](b7a8504)) * **gax-httpjson:** Add Post Quantum Cryptography (PQC) Support by default via Conscrypt ([#13853](#13853)) ([550df81](550df81)) * **gax-java:** add JSpecify Null annotations to gax ([#13799](#13799)) ([65aee08](65aee08)) * **google/cloud/sql:** onboard a new library ([#13864](#13864)) ([38e272e](38e272e)) * **google/maps/navconnect/v1:** onboard a new library ([#13927](#13927)) ([2256394](2256394)) * **maps-isochrones:** onboard v1 API ([#13817](#13817)) ([3037ab3](3037ab3)) * port secure_context testing support to executor proxy ([#13522](#13522)) ([0f81bf0](0f81bf0)) * **productregistry:** onboard v1 API ([#13816](#13816)) ([9517313](9517313)) * **storage:** allow checksum on appendable upload finalization ([#13833](#13833)) ([ddf9add](ddf9add)) * **storage:** enable App-Centric Observability (ACO) support in Otel ([#13248](#13248)) ([4329896](4329896)) ### Bug Fixes * **bigquery-jdbc:** Add PerConnectionHandler to list of excempted logging classes ([#13888](#13888)) ([50b3c24](50b3c24)) * **bigquery-jdbc:** add preferIPv4Stack to argLine for Kokoro reliability ([#13923](#13923)) ([d5e33d6](d5e33d6)) * **bigquery-jdbc:** add service resource transformer for standalone IT ([#13893](#13893)) ([dc80fe8](dc80fe8)) * **bigquery-jdbc:** align metadata methods error handling with spec ([#13793](#13793)) ([d85fb10](d85fb10)) * **bigquery-jdbc:** fix WriteAPI when running in restricted environment ([#13856](#13856)) ([66ba925](66ba925)) * **bigquery-jdbc:** refine temporal timezone coercion and PreparedStatement parameter setters ([#13813](#13813)) ([6f68c4d](6f68c4d)) * **bigquery-jdbc:** resolve `ITOpenTelemetryTest` pipeline and trace validation failures ([#13898](#13898)) ([c18141d](c18141d)) * **bigquery-jdbc:** resolve failing otel IT in nightly ([#13915](#13915)) ([ac79713](ac79713)) * **bigquery:** resultSet.getLong() does not truncate for large int64 values ([#13718](#13718)) ([bc19822](bc19822)) * **bigquery:** support optional fields in BigLakeConfiguration to prevent NPE on Iceberg/Lakehouse tables ([#13733](#13733)) ([e2cca4d](e2cca4d)) * **bigtable:** add materialized view routing param to ReadRows and Sa… ([#13918](#13918)) ([4ddf250](4ddf250)) * **bigtable:** bound SessionPoolImpl lock to prevent pod-wide wedge ([#13890](#13890)) ([ed87a68](ed87a68)) * **bigtable:** fix session creation leaks ([#13887](#13887)) ([d586d07](d586d07)) * **bigtable:** prevent ClientConfigurationManagerTest from wedging on… ([#13907](#13907)) ([725086d](725086d)) * **bigtable:** stop installing DirectpathEnforcer on the directpath pool ([#13880](#13880)) ([5f2e056](5f2e056)) * **bom:** make release-note-generation Java 8 compatible ([#13837](#13837)) ([bc18390](bc18390)) * **ci:** fix java-cloud-bom release-notes workflow errors ([#13682](#13682)) ([b679835](b679835)) * deprecate resource detector ([#13844](#13844)) ([aba4f01](aba4f01)) * **deps:** align logback versions and add java8 profile in storage ([#13678](#13678)) ([7e57092](7e57092)) * do not start stream with direct executor ([#13945](#13945)) ([630e790](630e790)) * fix java-cloud-bom README update workflow after monorepo migration ([#13892](#13892)) ([5b8e295](5b8e295)) * **oauth2_http:** Avoid retrying on 4xx errors during GCE metadata ping ([#13715](#13715)) ([537c16c](537c16c)) * regenerate ([#13714](#13714)) ([8a72860](8a72860)) * regenerate libraries ([#13703](#13703)) ([a29ea79](a29ea79)), refs [#13690](#13690) * **release:** handle missing release tags gracefully in release-note-generation ([#13795](#13795)) ([43005fb](43005fb)) * **release:** resolve first-party-dependencies SNAPSHOT in libraries-bom ([#13790](#13790)) ([d5bfe21](d5bfe21)) * **spanner:** avoid data race on DIRECTPATH_CHANNEL_CREATED by using volatile ([#13727](#13727)) ([1e05ea5](1e05ea5)) * **spanner:** prevent fastpath tablet routing flaps ([#13803](#13803)) ([4dabdac](4dabdac)) * **storage:** BidiAppendableUpload Takeover operation fixes ([#13776](#13776)) ([f2d0474](f2d0474)) * **storage:** correctly insert explicit nulls for json patch updates ([#13716](#13716)) ([4fb3f4b](4fb3f4b)) * update group id mapping ([#13698](#13698)) ([50a72e1](50a72e1)) * use a new managed channel builder when creating channels ([#13684](#13684)) ([a049999](a049999)) ### Performance Improvements * **bigquery-jdbc:** optimize getExportedKeys performance using hybrid metadata lookup ([#13734](#13734)) ([c9738ea](c9738ea)) ### Dependencies * Add Conscrypt to shared-deps ([#13838](#13838)) ([05ce6ce](05ce6ce)) * move conscrypt from third-party-dependencies POM to gax-java POM ([#13948](#13948)) ([1e634ec](1e634ec)) * **shared-deps:** migrate awaitility to shared-dependencies ([#13671](#13671)) ([abb91ef](abb91ef)) * **shared-deps:** switch conscrypt shared dependency to conscrypt-openjdk-uber ([#13845](#13845)) ([2e3f208](2e3f208)) * Update gRPC-Java to v1.82.2 ([#13877](#13877)) ([da228d8](da228d8)) * Update http-client to v2.2.0 ([#13854](#13854)) ([334a2c5](334a2c5)) * Update Protobuf-Java to v4.33.6 ([#13876](#13876)) ([5c6478c](5c6478c)) * Upgrade Guava to v33.6.0-jre ([#13875](#13875)) ([41a7a52](41a7a52)) ### Documentation * add ErrorProne and NullAway integration guide and JSpecify migration playbook ([#13882](#13882)) ([d5ea739](d5ea739)) * add JSpecify nullness guidelines to AGENTS.md ([#13881](#13881)) ([54b846b](54b846b)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prevent the session pool from wedged forever from a orphaned monitor.
Using a reentry lock so we can set a timeout when trying to aquire the lock. if it failed from the timeout, the pool would return unavailable, and fallback to the classic path, preventing traffic getting rejected forever.
We should implement self healing session pool in a follow up PR.