Merge pull request #85 from GoogleCloudPlatform/zero-low-watermark

Prepare for maxConcurrentStreamsLowWatermark to be 0.
GoogleCloudPlatform · Jun 12, 2021 · 17df189 · 17df189
2 parents 3a6fb6f + 42638fa
commit 17df189
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 11 deletions.
diff --git a/grpc-gcp/src/main/java/com/google/grpc/gcp/GcpManagedChannel.java b/grpc-gcp/src/main/java/com/google/grpc/gcp/GcpManagedChannel.java
@@ -192,8 +192,6 @@ public GcpManagedChannel(
       this.maxSize = poolSize;
     }
     this.delegateChannelBuilder = delegateChannelBuilder;
-    // Initialize the first delegate channel.
-    getChannelRef(null);
     this.options = options;
     initOptions();
     if (options.getResiliencyOptions() != null) {
@@ -887,7 +885,13 @@ private synchronized ChannelRef pickLeastBusyChannel(boolean forFallback) {
 
   @Override
   public synchronized String authority() {
-    return channelRefs.get(0).getChannel().authority();
+    if (channelRefs.size() > 0) {
+      return channelRefs.get(0).getChannel().authority();
+    }
+    final ManagedChannel channel = delegateChannelBuilder.build();
+    final String authority = channel.authority();
+    channel.shutdownNow();
+    return authority;
   }
 
   /**
@@ -997,7 +1001,8 @@ public synchronized ConnectivityState getState(boolean requestConnection) {
     } else if (shutdown > 0) {
       return ConnectivityState.SHUTDOWN;
     }
-    return null;
+    // When no channels are created yet it is also IDLE.
+    return ConnectivityState.IDLE;
   }
 
   /**

diff --git a/grpc-gcp/src/main/proto/google/grpc/gcp/proto/grpc_gcp.proto b/grpc-gcp/src/main/proto/google/grpc/gcp/proto/grpc_gcp.proto
@@ -36,6 +36,8 @@ message ChannelPoolConfig {
   // The low watermark of max number of concurrent streams in a channel.
   // New channel will be created once it get hit, until we reach the max size
   // of the channel pool.
+  // Values outside of [0..100] will be ignored. 0 = always create a new channel
+  // until max size is reached.
   uint32 max_concurrent_streams_low_watermark = 3;
 }
 

diff --git a/grpc-gcp/src/test/java/com/google/grpc/gcp/GcpManagedChannelTest.java b/grpc-gcp/src/test/java/com/google/grpc/gcp/GcpManagedChannelTest.java
@@ -96,7 +96,7 @@ public void testLoadApiConfigFile() throws Exception {
             GcpManagedChannelBuilder.forDelegateBuilder(builder)
                 .withApiConfigJsonFile(configFile)
                 .build();
-    assertEquals(1, gcpChannel.channelRefs.size());
+    assertEquals(0, gcpChannel.channelRefs.size());
     assertEquals(3, gcpChannel.getMaxSize());
     assertEquals(2, gcpChannel.getStreamsLowWatermark());
     assertEquals(3, gcpChannel.methodToAffinity.size());
@@ -117,16 +117,17 @@ public void testLoadApiConfigString() throws Exception {
             GcpManagedChannelBuilder.forDelegateBuilder(builder)
                 .withApiConfigJsonString(sb.toString())
                 .build();
-    assertEquals(1, gcpChannel.channelRefs.size());
+    assertEquals(0, gcpChannel.channelRefs.size());
     assertEquals(3, gcpChannel.getMaxSize());
     assertEquals(2, gcpChannel.getStreamsLowWatermark());
     assertEquals(3, gcpChannel.methodToAffinity.size());
   }
 
   @Test
   public void testGetChannelRefInitialization() throws Exception {
-    // Should have a managedchannel by default.
-    assertEquals(1, gcpChannel.channelRefs.size());
+    // Should not have a managedchannel by default.
+    assertEquals(0, gcpChannel.channelRefs.size());
+    // But once requested it's there.
     assertEquals(0, gcpChannel.getChannelRef(null).getAffinityCount());
     // The state of this channel is idle.
     assertEquals(ConnectivityState.IDLE, gcpChannel.getState(false));
@@ -177,8 +178,8 @@ public void testBindUnbindKey() throws Exception {
     gcpChannel.bind(cf1, Arrays.asList("key1"));
     gcpChannel.bind(cf2, Arrays.asList("key2"));
     gcpChannel.bind(cf1, Arrays.asList("key1"));
-    assertEquals(2, gcpChannel.channelRefs.get(1).getAffinityCount());
-    assertEquals(1, gcpChannel.channelRefs.get(2).getAffinityCount());
+    assertEquals(2, gcpChannel.channelRefs.get(0).getAffinityCount());
+    assertEquals(1, gcpChannel.channelRefs.get(1).getAffinityCount());
     assertEquals(2, gcpChannel.affinityKeyToChannelRef.size());
 
     // Unbind the affinity key.
@@ -187,8 +188,8 @@ public void testBindUnbindKey() throws Exception {
     gcpChannel.unbind(Arrays.asList("key1"));
     gcpChannel.unbind(Arrays.asList("key2"));
     assertEquals(0, gcpChannel.affinityKeyToChannelRef.size());
+    assertEquals(0, gcpChannel.channelRefs.get(0).getAffinityCount());
     assertEquals(0, gcpChannel.channelRefs.get(1).getAffinityCount());
-    assertEquals(0, gcpChannel.channelRefs.get(2).getAffinityCount());
   }
 
   @Test