Skip to content

Commit e61486f

Browse files
committed
chore: Update ocmments and refactor
1 parent 2be58da commit e61486f

3 files changed

Lines changed: 18 additions & 25 deletions

File tree

sdk-platform-java/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ChannelPool.java

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@
6969
* <p>Package-private for internal use.
7070
*/
7171
class ChannelPool extends ManagedChannel {
72-
private static final String CHANNEL_POOL_CONSECUTIVE_RESIZING_WARNING =
72+
static final String CHANNEL_POOL_CONSECUTIVE_RESIZING_WARNING =
7373
"Channel pool is repeatedly resizing. "
7474
+ "Consider adjusting `initialChannelCount` or `maxResizeDelta` to a more reasonable value. "
7575
+ "See https://docs.cloud.google.com/java/docs/troubleshooting to enable logging "
@@ -90,8 +90,8 @@ class ChannelPool extends ManagedChannel {
9090
private final String authority;
9191

9292
// The number of consecutive resize cycles to wait before logging a warning about repeated
93-
// resizing. This is an arbitrary value chosen to detect repeated requests for changes
94-
// (multiple continuous increase or decrease attempts) without being too sensitive.
93+
// resizing. This value was chosen to detect repeated requests for changes (multiple continuous
94+
// increase or decrease attempts) without being too sensitive.
9595
private static final int CONSECUTIVE_RESIZE_THRESHOLD = 5;
9696

9797
// Tracks the number of consecutive resize cycles where a resize actually occurred (either expand
@@ -290,7 +290,8 @@ private void resizeSafely() {
290290
* <li>Get the maximum number of outstanding RPCs since last invocation
291291
* <li>Determine a valid range of number of channels to handle that many outstanding RPCs
292292
* <li>If the current number of channel falls outside of that range, add or remove at most
293-
* {@link ChannelPoolSettings#MAX_RESIZE_DELTA} to get closer to middle of that range.
293+
* {@link ChannelPoolSettings#DEFAULT_MAX_RESIZE_DELTA} to get closer to middle of that
294+
* range.
294295
* </ul>
295296
*
296297
* <p>Not threadsafe, must be called under the entryWriteLock monitor
@@ -332,18 +333,19 @@ void resize() {
332333
dampenedTarget = currentSize + (int) Math.copySign(settings.getMaxResizeDelta(), delta);
333334
}
334335

335-
// We only count as "resized" if the thresholds are crossed and we actually attempt to scale.
336-
// Checking (dampenedTarget != currentSize) would cause false positives when the pool is within
337-
// bounds but not at the target, because the target aims for the middle of the bounds.
336+
// Only count as "resized" if the thresholds are crossed and Gax attempts to scale. Checking
337+
// that `dampenedTarget != currentSize` would cause false positives when the pool is within
338+
// bounds but not at the target (target aims for the middle of the bounds)
338339
boolean resized = (currentSize < minChannels || currentSize > maxChannels);
339340
if (resized) {
340341
consecutiveResizes++;
341342
} else {
342343
consecutiveResizes = 0;
343344
}
344345

345-
// Log warning only once when the threshold is reached to avoid spamming logs.
346-
// Using == instead of >= ensures we don't log on every subsequent resize cycle.
346+
// Log warning only once when the consecutive threshold is reached to avoid spamming logs. Log
347+
// message will repeat if the number of consecutive resizes resets (e.g. stabilizes for a bit).
348+
// However, aim to log once to ensure that this does not incur log spam.
347349
if (consecutiveResizes == CONSECUTIVE_RESIZE_THRESHOLD) {
348350
LOG.warning(CHANNEL_POOL_CONSECUTIVE_RESIZING_WARNING);
349351
}

sdk-platform-java/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ChannelPoolSettings.java

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public abstract class ChannelPoolSettings {
5959
static final Duration RESIZE_INTERVAL = Duration.ofMinutes(1);
6060

6161
/** The maximum number of channels that can be added or removed at a time. */
62-
static final int MAX_RESIZE_DELTA = 2;
62+
static final int DEFAULT_MAX_RESIZE_DELTA = 2;
6363

6464
/**
6565
* Threshold to start scaling down the channel pool.
@@ -96,8 +96,8 @@ public abstract class ChannelPoolSettings {
9696
* The maximum number of channels that can be added or removed at a time.
9797
*
9898
* <p>This setting limits the rate at which the channel pool can grow or shrink in a single resize
99-
* period. The default value is {@value #MAX_RESIZE_DELTA}. Increasing this value can help the
100-
* pool better handle sudden bursts or spikes in requests by allowing it to scale up faster.
99+
* period. The default value is {@value #DEFAULT_MAX_RESIZE_DELTA}. Increasing this value can help
100+
* the pool better handle sudden bursts or spikes in requests by allowing it to scale up faster.
101101
* Regardless of this setting, the number of channels will never exceed {@link
102102
* #getMaxChannelCount()}.
103103
*/
@@ -127,11 +127,7 @@ boolean isStaticSize() {
127127
return true;
128128
}
129129
// When the scaling threshold are not set
130-
if (getMinRpcsPerChannel() == 0 && getMaxRpcsPerChannel() == Integer.MAX_VALUE) {
131-
return true;
132-
}
133-
134-
return false;
130+
return getMinRpcsPerChannel() == 0 && getMaxRpcsPerChannel() == Integer.MAX_VALUE;
135131
}
136132

137133
public abstract Builder toBuilder();
@@ -143,7 +139,6 @@ public static ChannelPoolSettings staticallySized(int size) {
143139
.setMaxRpcsPerChannel(Integer.MAX_VALUE)
144140
.setMinChannelCount(size)
145141
.setMaxChannelCount(size)
146-
.setMaxResizeDelta(Math.min(MAX_RESIZE_DELTA, size))
147142
.build();
148143
}
149144

@@ -155,7 +150,7 @@ public static Builder builder() {
155150
.setMinRpcsPerChannel(0)
156151
.setMaxRpcsPerChannel(Integer.MAX_VALUE)
157152
.setPreemptiveRefreshEnabled(false)
158-
.setMaxResizeDelta(MAX_RESIZE_DELTA);
153+
.setMaxResizeDelta(DEFAULT_MAX_RESIZE_DELTA);
159154
}
160155

161156
@AutoValue.Builder

sdk-platform-java/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/ChannelPoolTest.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -784,9 +784,7 @@ void repeatedResizingLogsWarningOnExpand() throws Exception {
784784
pool.resize();
785785
assertThat(logHandler.getAllMessages()).hasSize(1);
786786
assertThat(logHandler.getAllMessages())
787-
.contains(
788-
"Channel pool is repeatedly resizing. Consider adjusting `initialChannelCount` or `maxResizeDelta` to a more reasonable value. "
789-
+ "See https://docs.cloud.google.com/java/docs/troubleshooting to enable logging and set `com.google.api.gax.grpc.ChannelPool.level=FINEST` to log the channel pool resize behavior.");
787+
.contains(ChannelPool.CHANNEL_POOL_CONSECUTIVE_RESIZING_WARNING);
790788

791789
// 6th resize, should not log again
792790
pool.resize();
@@ -837,9 +835,7 @@ void repeatedResizingLogsWarningOnShrink() throws Exception {
837835
// 5th resize, should log warning
838836
pool.resize();
839837
assertThat(logHandler.getAllMessages())
840-
.contains(
841-
"Channel pool is repeatedly resizing. Consider adjusting `initialChannelCount` or `maxResizeDelta` to a more reasonable value. "
842-
+ "See https://docs.cloud.google.com/java/docs/troubleshooting to enable logging and set `com.google.api.gax.grpc.ChannelPool.level=FINEST` to log the channel pool resize behavior.");
838+
.contains(ChannelPool.CHANNEL_POOL_CONSECUTIVE_RESIZING_WARNING);
843839
} finally {
844840
ChannelPool.LOG.removeHandler(logHandler);
845841
}

0 commit comments

Comments
 (0)