commit a71b8497089188bbdcdab859dcfc8f93e3523578
author Carmelo Cascone <carmelo@opennetworking.org> Mon Dec 17 17:47:50 2018 -0800
committer Carmelo Cascone <carmelo@opennetworking.org> Mon Dec 17 17:58:54 2018 -0800
tree d3d7133af9dbeb6fb8e098906f4350bff735f1b3
parent 1a37866929ca05acba71cdadad87dd563b6064dd

Removes the gRPC channel if an exception occurs while instantiating the client

Includes also other minor changes to gRPC channel creation/connection
process, such as:
- More compact logs showing the gRPC client key
- GrpcChannelController.connectChannel() now returns the same
 StatusRuntime exception, no need to wrap it in an IOException
- Wait for channel shutdown after initial connection error

Change-Id: Ib7d2b728b8c82d9f9b2097cffcebd31cac891b27

protocols/grpc/ctl/src/main/java/org/onosproject/grpc/ctl/GrpcChannelControllerImpl.java

diff --git a/protocols/grpc/ctl/src/main/java/org/onosproject/grpc/ctl/GrpcChannelControllerImpl.java b/protocols/grpc/ctl/src/main/java/org/onosproject/grpc/ctl/GrpcChannelControllerImpl.java
index 7b9290e..4ad3166 100644
--- a/protocols/grpc/ctl/src/main/java/org/onosproject/grpc/ctl/GrpcChannelControllerImpl.java
+++ b/protocols/grpc/ctl/src/main/java/org/onosproject/grpc/ctl/GrpcChannelControllerImpl.java
@@ -48,7 +48,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
 import java.util.Collection;
 import java.util.Dictionary;
 import java.util.HashSet;
@@ -115,8 +114,7 @@
 
     @Override
     public ManagedChannel connectChannel(GrpcChannelId channelId,
-                                         ManagedChannelBuilder<?> channelBuilder)
-            throws IOException {
+                                         ManagedChannelBuilder<?> channelBuilder) {
         checkNotNull(channelId);
         checkNotNull(channelBuilder);
 
@@ -137,8 +135,8 @@
             try {
                 doDummyMessage(channel);
             } catch (StatusRuntimeException e) {
-                channel.shutdownNow();
-                throw new IOException(e);
+                shutdownNowAndWait(channel, channelId);
+                throw e;
             }
             // If here, channel is open.
             channels.put(channelId, channel);
@@ -199,27 +197,28 @@
         lock.lock();
         try {
             final ManagedChannel channel = channels.remove(channelId);
-            if (channel == null) {
-                // Nothing to do.
-                return;
-            }
-
-            try {
-                if (!channel.shutdownNow()
-                        .awaitTermination(5, TimeUnit.SECONDS)) {
-                    log.error("Channel '{}' didn't terminate, although we " +
-                                      "triggered a shut down and waited",
-                              channelId);
-                }
-            } catch (InterruptedException e) {
-                log.warn("Channel {} didn't shut down in time", channelId);
-                Thread.currentThread().interrupt();
+            if (channel != null) {
+                shutdownNowAndWait(channel, channelId);
             }
         } finally {
             lock.unlock();
         }
     }
 
+    private void shutdownNowAndWait(ManagedChannel channel, GrpcChannelId channelId) {
+        try {
+            if (!channel.shutdownNow()
+                    .awaitTermination(5, TimeUnit.SECONDS)) {
+                log.error("Channel '{}' didn't terminate, although we " +
+                                  "triggered a shutdown and waited",
+                          channelId);
+            }
+        } catch (InterruptedException e) {
+            log.warn("Channel {} didn't shutdown in time", channelId);
+            Thread.currentThread().interrupt();
+        }
+    }
+
     @Override
     public Map<GrpcChannelId, ManagedChannel> getChannels() {
         return ImmutableMap.copyOf(channels);