feat(grpc-gcp): drain scaled-down channels

[rahul2393]

With dynamic scaling enabled, scale-down removed the longest-connected channels abruptly: the channel disappeared from the pool while callers still held it, affinity mappings were dropped without going through normal unbind accounting, and the removed channel was only shut down when a later scale-down tick happened
to observe zero active streams. RPCs and transaction affinities pinned to a removed channel could land on a closing delegate, and the picker could crash (IndexOutOfBoundsException / IllegalStateException) when the pool shrank
mid-scan.

Change

• Scale-down drains instead of dropping. removeChannels() picks the least-loaded channels first (fewest bound affinities, then age, break ties), deactivates each one before publishing the shorter active list, and moves it to the draining set. In-flight RPCs finish on the draining delegate; new picks skip inactive channels. Affinity-key release and drain-timer arming happen after the pool lock is released.

• Draining is one-way (matching the Go reference): a draining channel is never returned to the active pool; scale-up always builds a fresh channel. Removes the legacy pickChannelForReuse path.

• Idle-grace shutdown. A draining channel shuts down only after drainIdleGrace(new pool option, default 1 min) of inactivity with zero active streams. The last stream completion arms the timer directly instead of waiting for the next scale-down tick. Drain timers carry their own identity, so a stale timer that fires late cannot cancel or act on behalf of a newer one; timer bookkeeping is guarded by the pool monitor while delegateshutdown()always runs outside it. If the scheduler rejects a drain task, the channel is closed immediately rather than leaked; all pending timers are cancelled onshutdown()/shutdownNow()`.

• Affinity consistency. Keys bound to removed channels are released through the normal unbind path, keeping per-channel and aggregate affinity counts correct. Key lookup tolerates concurrent rebinds without leaking or regressing last-used timestamps (merge with Long::max), a mapping to an inactive channel is atomically replaced (looping until an active mapping or a fresh bind), and bind() returns the channel actually bound so callers never hand out a stale draining reference. Caller-owned affinity refs stay sticky on a draining channel until its delegate shuts down, preserving transaction affinity across scale-down.

• Picker hardening. Pool scans use the CopyOnWriteArrayList snapshot iterator (bounds-safe under concurrent shrink by construction), load comparisons go through getPickerLoad(), the overload watermark uses the configured maxConcurrentStreamsLowWatermark instead of the DEFAULT_MAX_STREAM constant, and an exhausted pool fails with a retryable UNAVAILABLE status instead of IllegalStateException. Scale-up sizes itself from active channels only, with division guards for transiently negative load sums.

• Readiness accounting deltas are computed under the channel lock but applied outside it, removing the lock-order inversion between the channel and pool monitors.

[gemini-code-assist]

Code Review

This pull request introduces channel draining functionality to GcpManagedChannel, allowing removed channels to gracefully shut down after their in-flight calls reach zero and a configured drain idle grace period elapses. It also updates channel selection, scale-down, and affinity binding logic to support draining channels, and adds comprehensive unit tests. The review feedback highlights a potential race condition when updating the affinity key last-used timestamp concurrently, as well as a redundant null check in the channel selection logic.

[rahul2393]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a channel draining mechanism to GcpManagedChannel, allowing channels to scale down gracefully by waiting for in-flight RPCs to complete and respecting a configurable idle grace period before shutting down. It also updates affinity key bindings and channel selection strategies to prevent routing new requests to draining channels. The feedback recommends replacing the index-based loops and the candidateAt helper with standard enhanced for loops to leverage CopyOnWriteArrayList's thread-safe snapshot iterator, which avoids potential IndexOutOfBoundsExceptions. Additionally, it is suggested to consolidate the overloaded leastLoadedActiveChannel methods into a single method accepting Iterable.

[rahul2393]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a channel draining mechanism to GcpManagedChannel, allowing removed channels to gracefully shut down after their in-flight streams complete and a configurable idle grace period (drainIdleGrace) elapses. It also enables the reuse of these draining channels during subsequent scale-up events and improves the concurrency safety of affinity key bindings. A high-severity issue was identified in the channel selection strategy (pickFromCandidates), where concurrent shrinking of the channel list during scale-down could lead to an IndexOutOfBoundsException; copying the candidates to a local snapshot before indexing was suggested to resolve this.

[rahul2393]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a channel draining mechanism in GcpManagedChannel to gracefully scale down channels, adding a configurable drainIdleGrace option and replacing channel reuse with fresh channel creation during scale-up. It also refactors channel selection and affinity key mapping to handle inactive channels and concurrent updates more robustly, supported by a new suite of draining tests. The review feedback highlights a concurrency issue where a concurrent rebind could be overwritten during inactive channel checks, suggesting that unbindInactiveMapping return a boolean to safely handle this case. Additionally, the reviewer recommends updating the Javadoc for setDrainIdleGrace to clarify that draining channels remain open for existing affinity rather than being reusable.

[rahul2393]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements a channel draining mechanism with an idle grace period to replace the previous channel reuse logic, ensuring that removed channels are safely shut down after their in-flight streams complete. The reviewer feedback highlights several critical concurrency and performance improvements. These include using a while loop instead of an if statement when unbinding inactive mappings to handle concurrent updates, introducing a DrainTask wrapper to prevent race conditions between scheduled drain tasks, and moving the affinityKeyToChannelRef scan outside of the synchronized block to minimize lock contention.

[rahul2393]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a channel draining mechanism to GcpManagedChannel to handle scaling down more gracefully. Instead of immediately shutting down removed channels, they are now transitioned to a draining state where they wait for a grace period (drainIdleGrace) and for in-flight streams to complete before closing. It also updates channel selection strategies (like power-of-two) to avoid picking draining channels, and refactors affinity key binding/unbinding to handle draining channels. The review feedback highlights two important issues: a potential concurrency bug in pickFromCandidates where accessing candidates via index can throw IndexOutOfBoundsException if the list shrinks concurrently during scale-down, and a potential issue in ceilDiv where negative numerators are not handled correctly.

[rahul2393]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a channel draining mechanism in GcpManagedChannel to handle scaling down more gracefully. Instead of immediately shutting down removed channels, they are marked as draining and kept open for a configurable grace period (drainIdleGrace) to allow in-flight streams to complete and sticky affinity references to remain valid. The changes also include refactoring channel selection, scaling logic, and adding comprehensive unit tests. The review feedback highlights two key issues: first, the bind method should return the bound ChannelRef to prevent callers from returning inactive channels when a replacement occurs; second, the active channel count in dynamicUpscale should only include active channels to ensure accurate scale-up calculations.

[rahul2393]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a graceful draining mechanism for channels during scale-down in GcpManagedChannel. Instead of immediately shutting down removed channels, they are now marked as draining and remain open for a configurable drainIdleGrace period to allow in-flight calls to complete and sticky affinity references to remain valid. The scale-down selection strategy has been improved to prefer removing least-loaded channels first (breaking ties with fewer affinity bindings and older allocations), and scale-up now always builds fresh channels instead of reusing draining ones. Additionally, test coverage has been significantly enhanced with the introduction of GcpManagedChannelDrainingTest and the replacement of flaky sleep calls with awaitility and a mock clock. As there are no review comments, no further feedback is provided.

[olavloite]

There is a very small chance of a concurrency issue here:

1. We get here when the channel is not shut down, but it could be draining. Let us assume that it is draining.
2. We return the channelRef here.
3. Before the caller can actually execute an RPC, the background task that shuts down draining channels runs and the grace period has just been reached, then the channel will be shut down.
4. The caller that received the channelRef tries to execute an RPC, but the channel has been shut down, so an exception is thrown.

However, with default settings (60s drain grace period), the probability of this happening is close to zero. If the grace period for whatever reason is configured to be much lower (or even zero), then there is a real possibility that this could happen.