fix: fix dynamic flow control setting checks (#1347)

* fix: fix dynamic flow control setting checks * make checks more readable
googleapis · Apr 21, 2021 · 69458b4 · 69458b4
1 parent 06dbf12
commit 69458b4
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 10 deletions.
diff --git a/gax/src/main/java/com/google/api/gax/batching/DynamicFlowControlSettings.java b/gax/src/main/java/com/google/api/gax/batching/DynamicFlowControlSettings.java
@@ -115,11 +115,14 @@ public DynamicFlowControlSettings build() {
     }
 
     private void verifyElementCountSettings(DynamicFlowControlSettings settings) {
-      boolean isEnabled =
-          settings.getInitialOutstandingElementCount() != null
-              || settings.getMinOutstandingElementCount() != null
-              || settings.getMaxOutstandingElementCount() != null;
-      if (!isEnabled) {
+      // If LimitExceededBehavior is Ignore, dynamic flow control is disabled, there's no need to
+      // check element count limit settings
+      if (settings.getLimitExceededBehavior() == LimitExceededBehavior.Ignore) {
+        return;
+      }
+      if (settings.getInitialOutstandingElementCount() == null
+          && settings.getMinOutstandingElementCount() == null
+          && settings.getMaxOutstandingElementCount() == null) {
         return;
       }
       Preconditions.checkState(
@@ -141,11 +144,14 @@ private void verifyElementCountSettings(DynamicFlowControlSettings settings) {
     }
 
     private void verifyRequestBytesSettings(DynamicFlowControlSettings settings) {
-      boolean isEnabled =
-          settings.getInitialOutstandingRequestBytes() != null
-              || settings.getMinOutstandingRequestBytes() != null
-              || settings.getMaxOutstandingRequestBytes() != null;
-      if (!isEnabled) {
+      // If LimitExceededBehavior is Ignore, dynamic flow control is disabled, there's no need to
+      // check request bytes limit settings
+      if (settings.getLimitExceededBehavior() == LimitExceededBehavior.Ignore) {
+        return;
+      }
+      if (settings.getInitialOutstandingRequestBytes() == null
+          && settings.getMinOutstandingRequestBytes() == null
+          && settings.getMaxOutstandingRequestBytes() == null) {
         return;
       }
       Preconditions.checkState(

diff --git a/gax/src/test/java/com/google/api/gax/batching/DynamicFlowControlSettingsTest.java b/gax/src/test/java/com/google/api/gax/batching/DynamicFlowControlSettingsTest.java
@@ -55,6 +55,22 @@ public void testEmptyBuilder() {
     assertEquals(LimitExceededBehavior.Block, settings.getLimitExceededBehavior());
   }
 
+  @Test
+  public void testPartialSettingsIgnored() {
+    // If behavior is ignore, build shouldn't throw exceptions even when only one of the bytes or
+    // element limits is set
+    DynamicFlowControlSettings.Builder builder =
+        DynamicFlowControlSettings.newBuilder()
+            .setLimitExceededBehavior(LimitExceededBehavior.Ignore)
+            .setMaxOutstandingElementCount(1L);
+    builder.build();
+    builder =
+        DynamicFlowControlSettings.newBuilder()
+            .setLimitExceededBehavior(LimitExceededBehavior.Ignore)
+            .setMinOutstandingRequestBytes(1L);
+    builder.build();
+  }
+
   @Test
   public void testBuilder() {
     DynamicFlowControlSettings.Builder builder =