feat(spanner): prime scaled-up dynamic channel pool channels with SELECT 1 - #14254
Conversation
…ECT 1 The grpc-gcp dynamic channel pool (DCP) now supports a GcpChannelPrimer that runs before a scaled-up channel is published to the pool, but java-spanner never registered one, so channels added under load were published cold: the first requests on them paid for the TCP handshake, TLS session, and server-side connection setup at the moment the pool was already saturated. The Go Spanner client primes scaled-up channels by executing SELECT 1 with the multiplexed session through the new channel. This change brings java-spanner to parity.
There was a problem hiding this comment.
Code Review
This pull request implements channel priming for the dynamic channel pool in the Java Spanner client. It introduces the DynamicChannelPoolPrimer class to execute SELECT 1 on newly scaled-up channels using multiplexed sessions, ensuring connections are established before serving live traffic. It also adds owner ticket registration to safely associate multiplexed sessions with active database clients, and updates SpannerOptions and GapicSpannerRpc to support and configure this priming behavior. Feedback on the pull request points out missing imports for CallCredentials and MoreCallCredentials in GapicSpannerRpc.java, which would cause compilation failures.
| import com.google.auth.Credentials; | ||
| import com.google.cloud.RetryHelper; | ||
| import com.google.cloud.RetryHelper.RetryHelperException; | ||
| import com.google.cloud.grpc.GcpChannelPrimer; |
There was a problem hiding this comment.
The classes io.grpc.CallCredentials and io.grpc.auth.MoreCallCredentials are used in createChannelPrimer but are not imported in this file, which will cause a compilation error. Please add the missing imports.
| import com.google.cloud.grpc.GcpChannelPrimer; | |
| import com.google.cloud.grpc.GcpChannelPrimer; | |
| import io.grpc.CallCredentials; | |
| import io.grpc.auth.MoreCallCredentials; |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements a dynamic channel pool priming mechanism for the Google Cloud Spanner Java client. When dynamic channel pooling is enabled, newly scaled-up channels are primed by executing a SELECT 1 query using a multiplexed session before they are published to the pool, ensuring transport, TLS, and server-side connections are established. The changes introduce the DynamicChannelPoolPrimer class, integrate it with MultiplexedSessionDatabaseClient and GapicSpannerRpc, and add corresponding configuration options and tests. The review feedback highlights two important improvements: first, lazily retrieving credentials inside the CallCredentialsProvider lambda to prevent blocking the startup thread or failing client creation eagerly; second, wrapping the priming execution in a try-catch block to ensure any synchronous exceptions are safely propagated as failed futures.
| static CallCredentialsProvider createChannelPrimeCallCredentialsProvider( | ||
| CredentialsProvider credentialsProvider, | ||
| @Nullable CallCredentialsProvider callCredentialsProvider) { | ||
| final CallCredentials defaultCallCredentials; | ||
| try { | ||
| Credentials credentials = credentialsProvider.getCredentials(); | ||
| defaultCallCredentials = credentials == null ? null : MoreCallCredentials.from(credentials); | ||
| } catch (IOException e) { | ||
| throw newSpannerException(e); | ||
| } | ||
| if (callCredentialsProvider == null) { | ||
| return defaultCallCredentials == null ? null : () -> defaultCallCredentials; | ||
| } | ||
| return () -> { | ||
| CallCredentials callCredentials = callCredentialsProvider.getCallCredentials(); | ||
| return callCredentials != null ? callCredentials : defaultCallCredentials; | ||
| }; | ||
| } |
There was a problem hiding this comment.
Eagerly calling credentialsProvider.getCredentials() during RPC initialization can block the startup thread or fail client creation if there is a transient issue or if the credentials provider is not yet ready. It also prevents dynamic/custom credentials providers from returning fresh credentials if they are swapped or updated, as the first instance is cached forever.
Making the call to credentialsProvider.getCredentials() lazy inside the returned CallCredentialsProvider lambda resolves these issues and improves robustness.
| static CallCredentialsProvider createChannelPrimeCallCredentialsProvider( | |
| CredentialsProvider credentialsProvider, | |
| @Nullable CallCredentialsProvider callCredentialsProvider) { | |
| final CallCredentials defaultCallCredentials; | |
| try { | |
| Credentials credentials = credentialsProvider.getCredentials(); | |
| defaultCallCredentials = credentials == null ? null : MoreCallCredentials.from(credentials); | |
| } catch (IOException e) { | |
| throw newSpannerException(e); | |
| } | |
| if (callCredentialsProvider == null) { | |
| return defaultCallCredentials == null ? null : () -> defaultCallCredentials; | |
| } | |
| return () -> { | |
| CallCredentials callCredentials = callCredentialsProvider.getCallCredentials(); | |
| return callCredentials != null ? callCredentials : defaultCallCredentials; | |
| }; | |
| } | |
| static CallCredentialsProvider createChannelPrimeCallCredentialsProvider( | |
| CredentialsProvider credentialsProvider, | |
| @Nullable CallCredentialsProvider callCredentialsProvider) { | |
| return () -> { | |
| if (callCredentialsProvider != null) { | |
| CallCredentials callCredentials = callCredentialsProvider.getCallCredentials(); | |
| if (callCredentials != null) { | |
| return callCredentials; | |
| } | |
| } | |
| try { | |
| Credentials credentials = credentialsProvider.getCredentials(); | |
| return credentials == null ? null : MoreCallCredentials.from(credentials); | |
| } catch (IOException e) { | |
| throw newSpannerException(e); | |
| } | |
| }; | |
| } |
References
- Prefer lazy initialization over eager initialization for resource-intensive or blocking operations if they are not guaranteed to be used immediately, to avoid unnecessary performance overhead.
There was a problem hiding this comment.
constructor already resolves credentials eagerly through GAX
| @Override | ||
| public ListenableFuture<Void> prime(ManagedChannel channel) { | ||
| String sessionName = getPrimeSessionName(); | ||
| if (sessionName == null) { | ||
| // The primer cannot gate the pool's scale-up decision, so the attempt fails fast. The pool's | ||
| // retry with backoff and its close-on-failure behaviour handle the unavailable session. | ||
| return Futures.immediateFailedFuture( | ||
| SpannerExceptionFactory.newSpannerException( | ||
| ErrorCode.FAILED_PRECONDITION, | ||
| "Cannot prime a dynamic channel pool channel before a multiplexed session is" | ||
| + " available")); | ||
| } | ||
| return executePrimeStatement(channel, sessionName); | ||
| } |
There was a problem hiding this comment.
The prime method returns a ListenableFuture<Void>, but executePrimeStatement can throw synchronous exceptions (e.g., if callCredentialsProvider.getCallCredentials() throws a SpannerException or if metadata/headers generation fails). Synchronous exceptions from future-returning methods can bypass standard future error handling and crash calling threads/executors.
Wrapping the call to executePrimeStatement in a try-catch block and returning Futures.immediateFailedFuture(t) ensures that all failures are propagated safely as failed futures.
@Override
public ListenableFuture<Void> prime(ManagedChannel channel) {
String sessionName = getPrimeSessionName();
if (sessionName == null) {
// The primer cannot gate the pool's scale-up decision, so the attempt fails fast. The pool's
// retry with backoff and its close-on-failure behaviour handle the unavailable session.
return Futures.immediateFailedFuture(
SpannerExceptionFactory.newSpannerException(
ErrorCode.FAILED_PRECONDITION,
"Cannot prime a dynamic channel pool channel before a multiplexed session is"
+ " available"));
}
try {
return executePrimeStatement(channel, sessionName);
} catch (Throwable t) {
return Futures.immediateFailedFuture(t);
}
}References
- Ensure that any future-returning method guarantees completion (either successfully or exceptionally) even if synchronous exceptions are thrown during initialization. Wrap the execution in a try-catch block to handle failures and return a failed future.
There was a problem hiding this comment.
It already catches Throwable
Ensure the primer API boundary converts synchronous failures into failed futures. Restore the built-in metrics test to the merged upstream emulator-host setup.
The grpc-gcp dynamic channel pool (DCP) now supports a GcpChannelPrimer that runs before a scaled-up channel is published to the pool, but java-spanner never registered one, so channels added under load were published cold: the first requests on them paid for the TCP handshake, TLS session, and server-side connection setup at the moment the pool was already saturated. The Go Spanner client primes scaled-up channels by executing SELECT 1 with the multiplexed session through the new channel. This change brings java-spanner to parity.