Skip to content

feat(grpc-gcp): prime scaled channels before publish - #14232

Open
rahul2393 wants to merge 1 commit into
mainfrom
fm/dcp-split-5-channel-primer
Open

feat(grpc-gcp): prime scaled channels before publish#14232
rahul2393 wants to merge 1 commit into
mainfrom
fm/dcp-split-5-channel-primer

Conversation

@rahul2393

Copy link
Copy Markdown
Contributor

With dynamic scaling enabled, a freshly built channel was published to the pool the moment its delegate was constructed. Its first real RPCs then paid for connection establishment, TLS, and any service-side warm-up, so every scale-up event injected a latency spike into live traffic right when the pool was already under load.

Change

  • Optional channel primer. New GcpChannelPrimer hook (setChannelPrimer) issues a cheap end-to-end RPC on each newly built scale-up channel before it is published; for Cloud Spanner this can be a SELECT 1. Channels in a scale-up batch are primed concurrently and each one is published as soon as its own prime succeeds, so one slow channel never holds back the rest. A null primer (the default) keeps the existing publish-on-build path. Only dynamic scale-up channels are primed; the initial pool is not.
  • Bounded attempts. Each attempt is bounded by channelPrimeTimeout (default 10s) and retried up to channelPrimeMaxAttempts (default 3) with exponential backoff (100ms doubling, capped at 5s). The timeout covers the whole attempt, including the synchronous prime() call. A primer still blocked inside prime() at the timeout fails the channel without a retry, since nothing can stop that call; primer invocations run on a dedicated executor so a blocked primer cannot starve the shared scheduler that drives scale-up, draining, and timeouts. Exhausted or timed-out channels are closed and counted in the new scale_up_prime_failures metric.
  • Scale-up accounting. Channels still priming count toward both the pool size cap and the desired size, so a second scale-up during a slow prime batch cannot rebuild the same shortfall or construct delegates past maxSize. A prime abandoned because its call never returned keeps its slot until the call does, so a stuck primer can pin at most maxSize slots rather than one thread per scale-up event.
  • Lifecycle. Unpublished priming channels are never visible to pickers; shutdown cancels every pending prime and closes its delegate. Per-RPC pick and stream-accounting paths are unchanged, with no new hot-path allocations.
  • Configuration. setChannelPrimeTimeout rejects negative or non-nanosecond-representable durations (zero uses the default); setChannelPrimeMaxAttempts rejects negatives (zero uses the default).

@rahul2393
rahul2393 requested review from a team as code owners September 1, 2026 19:00

@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 introduces a channel priming mechanism (GcpChannelPrimer) to warm up newly built delegate channels during dynamic scale-up before they are published to the channel pool. It adds options to configure the primer, timeout, and maximum retry attempts, along with a dedicated cached thread pool to execute priming tasks asynchronously without blocking the shared background scheduler. Additionally, it updates the dynamic scale-up logic to account for in-flight priming channels to prevent over-provisioning, integrates a new metric (scale_up_prime_failures) to track priming failures, and includes comprehensive unit tests validating the priming lifecycle, timeouts, retries, and shutdown behavior. There are no review comments, so I have no feedback to provide.

/** A channel management factory that implements grpc.Channel APIs. */
public class GcpManagedChannel extends ManagedChannel {

private static final class PendingPrime {

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.

Can we move both this inner class and most (all?) of the logic that is being added here to a separate file instead of adding even more to GcpManagedChannel? Or is it strictly required that we add it here?

Comment on lines +405 to +408
@VisibleForTesting
int inFlightPrimeCountForTest() {
return inFlightPrimeCount.get();
}

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.

nit: This can probably just return pendingPrimes.size(). But can you also check whether we really need both this and the other methods here that are only added for tests?

}

private static long primeBackoffMillis(int attempt) {
return Math.min(100L << Math.min(attempt, 12), 5000L);

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.

Can we add some random jitter here to spread the retries a bit when we are creating multiple channels at the same time?

Comment on lines +29 to +42
/**
* Issues a cheap end-to-end RPC on {@code channel} so its connection is warm before real traffic.
* Scale-up batches invoke this method concurrently across channels and publish each channel as
* soon as its own future succeeds. For example, a Cloud Spanner implementation can execute {@code
* SELECT 1}. Return a failed future to reject the attempt; the pool retries up to the configured
* attempt count and closes the channel when they are exhausted.
*
* <p>Return promptly and do the work inside the future. The configured prime timeout bounds each
* attempt as a whole: a future still pending at the timeout is cancelled and the attempt retried,
* while a call that is still blocked inside this method at the timeout fails the channel with no
* retry, because nothing can stop it. A retry may overlap work from a cancelled future if that
* work does not promptly honor cancellation, so implementations must tolerate concurrent calls
* for one channel.
*/

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.

Can we extend this comment a bit further to explain what implementers need to take into account when implementing this method. Something like this (this came up when I started worrying and asking questions around the fact that Spanner's ExecuteStreamingSql has a default 1hr timeout):

  /**
   * Issues a cheap, non-mutating end-to-end RPC on {@code channel} so that its underlying transport,
   * TLS session, and server-side connections are fully established before the channel serves live traffic.
   *
   * <p>Scale-up batches invoke this method concurrently across channels, and each channel is published
   * to the pool as soon as its own future completes successfully. For example, a Cloud Spanner
   * implementation might execute {@code SELECT 1}. Returning a failed future rejects the attempt;
   * the pool retries up to the configured max attempts before discarding and closing the channel.
   *
   * <h3>Implementation Requirements &amp; Best Practices:</h3>
   * <ul>
   *   <li><b>Return promptly — Do NOT block:</b> This method must return a {@link ListenableFuture}
   *       immediately without performing synchronous or blocking work (e.g., avoid {@code blockingStub},
   *       {@code ResultSet.next()}, {@code Future.get()}, or thread synchronization). Blocking inside
   *       this method starves the primer executor and, if timed out, pins pool scale-up capacity.</li>
   *
   *   <li><b>Set an explicit, short RPC deadline:</b> The underlying gRPC call <em>MUST</em> be
   *       configured with an explicit, short deadline (e.g., {@code stub.withDeadlineAfter(5, SECONDS)}).
   *       Many Google Cloud APIs (such as Cloud Spanner's streaming queries) have default deadlines of
   *       up to 1 hour. Never rely on default RPC deadlines; a slow or unresponsive backend could leave
   *       calls lingering indefinitely on the server.</li>
   *
   *   <li><b>Propagate cancellation to the gRPC call:</b> When the pool's configured prime timeout
   *       expires, it invokes {@code future.cancel(true)} on the returned future. Implementers must
   *       ensure that cancelling this future actively cancels the underlying gRPC {@link io.grpc.ClientCall}
   *       or {@link io.grpc.Context.CancellableContext} so transport streams and server-side resources
   *       are terminated immediately.</li>
   *
   *   <li><b>Tolerate concurrent calls:</b> If a cancelled RPC does not terminate immediately on the
   *       wire, a subsequent retry attempt may be issued while the previous attempt is still winding down.
   *       Implementations must be safe for multiple concurrent invocations on the same channel.</li>
   *
   *   <li><b>Use trivial, read-only operations:</b> The operation should be strictly read-only,
   *       side-effect free, and require minimal server compute (e.g., {@code SELECT 1} or a lightweight ping).</li>
   * </ul>
   *
   * @param channel the newly created, unpublished delegate channel to prime
   * @return a {@link ListenableFuture} that completes when the priming RPC succeeds, or fails to trigger a retry
   */
  ListenableFuture<Void> prime(ManagedChannel channel);

if (blockedInPrimer || shuttingDown || attempt + 1 >= channelPrimeMaxAttempts) {
finishPendingPrime(pendingPrime);
if (blockedInPrimer && !shuttingDown) {
abandonedPrimes.add(pendingPrime);

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.

Should we log a warning for this? It happens completely in the background and is invisible to the application (I know that we have a metric for it, but not everyone is collecting that, and it would be a short blip in the overall metrics if it only happens a few times)

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