Skip to content

fix(bigtable): handle empty SessionPoolConfiguration in SessionPoolImpl and PoolSizer - #14912

Open
igorbernstein2 wants to merge 1 commit into
mainfrom
fix-session-pool-disabled-config
Open

igorbernstein2 wants to merge 1 commit into
mainfrom
fix-session-pool-disabled-config

Conversation

@igorbernstein2

Copy link
Copy Markdown
Contributor

Summary

When session_load == 0 across all scopes, ClientConfigurationManager.normalizeConfig clears session_configuration, resulting in an empty SessionPoolConfiguration.getDefaultInstance().

This change updates PoolSizer and SessionPoolImpl to handle an empty SessionPoolConfiguration cleanly:

  • PoolSizer:
    • Guards pendingVRpcsPerSession > 0 in getScaleDelta() to prevent divide-by-zero (+Infinity -> Integer.MAX_VALUE and int overflow when inUseCount >= 1).
    • Updates handleGoAway and handleSessionClose from getScaleDelta() >= 0 to getScaleDelta() > 0 (since onSessionClosing / onSessionClosed already decrements expectedCapacity prior to consulting PoolSizer).
  • SessionPoolImpl & SessionList:
    • Guards maybeScheduleCreateSessionRetry() with budget.getMaxConcurrentRequest() <= 0 so it does not spin on a 1ms retry timer when budget == 0.
    • Makes SessionHandle.onSessionClosing() idempotent with respect to expectedCapacity.
    • In the config listener, when newConfig is SessionPoolConfiguration.getDefaultInstance(), cancels retryCreateSessionFuture and gracefully closes all sessions (CLOSE_SESSION_REASON_DOWNSIZE); when newConfig is non-empty and poolState == PoolState.STARTED, scales back up via the pre-start loop.

@igorbernstein2
igorbernstein2 requested review from a team as code owners November 3, 2026 18:36

@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 adds robust handling for empty or disabled session pool configurations in the Google Cloud Bigtable client. Key changes include preventing division-by-zero errors in PoolSizer when pendingVRpcsPerSession is zero, updating SessionPoolImpl to properly cancel retry futures and close active sessions when the configuration is reset to default, and adding corresponding unit tests. The review feedback correctly identifies a compilation error in SessionPoolImpl.java where Future.cancel() is called without its required boolean argument, and suggests using cancel(false) instead.

if (newConfig.equals(
SessionClientConfiguration.SessionPoolConfiguration.getDefaultInstance())) {
if (retryCreateSessionFuture != null) {
retryCreateSessionFuture.cancel();

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.

critical

In Java, Future.cancel (and its sub-interfaces like ScheduledFuture) requires a boolean argument (mayInterruptIfRunning) to specify whether the executing thread should be interrupted. Calling cancel() with no arguments will result in a compilation error. Please use cancel(false) instead.

Suggested change
retryCreateSessionFuture.cancel();
retryCreateSessionFuture.cancel(false);

This branch has not been deployed

No deployments
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.

1 participant