feat(gax-grpc): add configurable resize delta and warning for repeated resizing#12838
feat(gax-grpc): add configurable resize delta and warning for repeated resizing#12838
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a configurable maxResizeDelta for the ChannelPool and adds a warning log when the pool resizes consecutively for five cycles. Feedback highlights a logic error in the resizing detection that could lead to false-positive warnings when the pool is at its limits; a suggestion is provided to track actual size changes instead. Additionally, the removal of test exclusions in the pom.xml was flagged as a potential cause for CI failures.
I am having trouble creating individual review comments. Click here to see my feedback.
sdk-platform-java/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ChannelPool.java (322-327)
The logic for determining if the pool is 'resizing' is flawed. By checking if the current size is outside the load-based bounds (minChannels and maxChannels), the counter will increment even when the pool is at its hard limits (e.g., at minChannelCount while idle). This will lead to false positive warnings after 5 idle cycles because minChannels will be 0 while the pool is correctly clamped at its minimum size (e.g., 1).
Instead, you should track whether the pool is actually attempting to change its size by comparing dampenedTarget with currentSize. This correctly identifies when the pool is either oscillating or slowly growing/shrinking due to the maxResizeDelta cap.
boolean resized = (dampenedTarget != currentSize);
if (resized) {
consecutiveResizes++;
} else {
consecutiveResizes = 0;
}
sdk-platform-java/gax-java/gax-grpc/pom.xml (145-149)
The removal of the maven-surefire-plugin configuration seems unintended. This block was used to exclude specific tests that require environment variables to be set. Removing it will likely cause these tests to run and fail in standard CI environments. If this change was intentional, please provide justification; otherwise, it should be reverted.
b17aa56 to
827b22d
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a configurable maxResizeDelta for the ChannelPool in gax-grpc and implements a warning mechanism for repeated resizing cycles. Feedback focuses on ensuring thread-safety for the resize counter, optimizing redundant size checks, and restoring accidentally deleted metadata and documentation links in the java-iam-policy module.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a configurable maxResizeDelta in ChannelPoolSettings, adds logic to ChannelPool to track consecutive resizes and log warnings, and removes certain metadata and documentation links. Feedback indicates that the removal of the enable-api link in the README appears accidental and should be restored. Additionally, a stale comment in ChannelPool.java incorrectly describing the resize() method as synchronized should be removed.
No description provided.