Repository: drill
Updated Branches:
  refs/heads/master 894c0f58e -> 1c14d3c3c


DRILL-6090: While connecting to drill-bits using JDBC Driver through Zookeeper, 
a lot of "Curator-Framework-0" threads are created if connection to drill-bit 
is not successful(no drill-bits are up/reachable)

I am using Drill JDBC driver 1.12.0 to connect to MapR-DB. I am finding the 
available drill-bits using Zookeepers. When drill-bits are not up or not 
reachable, the connection is failed with exception: "Failure in connecting to 
Drill: oadd.org.apache.drill.exec.rpc.RpcException: Failure setting up ZK for 
client", which is expected, but number of threads created by 
ZKClusterCoordinator just keeps on increasing.

Steps to reproduce the issue

Setup a connection with a drill-bit using Apache Drill JDBC driver 1.12.0 
through Zookeeper hosts(port 5181)
Now stop the drill-bit services or block the drill-bit IPs using iptable rules
Truncate catalina logs
Try to connect to the drill-bit/hit a code path that requires connection to 
drill-bits.
Take thread dump using kill -QUIT <java process id>
grep -c "Curator-Framework-0" catalina.out
Observe that the curator framework thread just keep on accumulating

RCA:

ZKClusterCoordinator creates curator threads in the constructor
ZKClusterCoordinator is instantiated by DrillClient.connect
DrillClient.connect is called in DrillConnectionImpl constructor

Fix:

Call DrillConnectionImpl .cleanup() from all the catch blocks in the 
DrillConnectionImpl  constructor.

close apache/drill#1094


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/8cb32343
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/8cb32343
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/8cb32343

Branch: refs/heads/master
Commit: 8cb32343b2816fe97f04efc055e6ffba77a9116a
Parents: 894c0f5
Author: milindt <35451050+mili...@users.noreply.github.com>
Authored: Wed Jan 17 14:35:33 2018 +0530
Committer: Aman Sinha <asi...@maprtech.com>
Committed: Fri Feb 23 14:08:30 2018 -0800

----------------------------------------------------------------------
 .../drill/jdbc/impl/DrillConnectionImpl.java    | 130 ++++++++++---------
 1 file changed, 69 insertions(+), 61 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/8cb32343/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillConnectionImpl.java
----------------------------------------------------------------------
diff --git 
a/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillConnectionImpl.java 
b/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillConnectionImpl.java
index 689041c..4c1ac46 100644
--- 
a/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillConnectionImpl.java
+++ 
b/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillConnectionImpl.java
@@ -110,73 +110,81 @@ class DrillConnectionImpl extends AvaticaConnection
     super.setReadOnly(false);
 
     this.config = new DrillConnectionConfig(info);
+    try{
+      try {
+        String connect = null;
 
-    try {
-      String connect = null;
-
-      if (config.isLocal()) {
-        try {
-          Class.forName("org.eclipse.jetty.server.Handler");
-        } catch (final ClassNotFoundException e) {
-          throw new SQLNonTransientConnectionException(
-              "Running Drill in embedded mode using Drill's jdbc-all JDBC"
-              + " driver Jar file alone is not supported.",  e);
-        }
-
-        final DrillConfig dConfig = DrillConfig.create(info);
-        this.allocator = RootAllocatorFactory.newRoot(dConfig);
-        RemoteServiceSet set = GlobalServiceSetReference.SETS.get();
-        if (set == null) {
-          // We're embedded; start a local drill bit.
-          serviceSet = RemoteServiceSet.getLocalServiceSet();
-          set = serviceSet;
+        if (config.isLocal()) {
           try {
-            bit = new Drillbit(dConfig, serviceSet);
-            bit.run();
-          } catch (final UserException e) {
-            throw new SQLException(
-                "Failure in starting embedded Drillbit: " + e.getMessage(),
-                e);
-          } catch (Exception e) {
-            // (Include cause exception's text in wrapping exception's text so
-            // it's more likely to get to user (e.g., via SQLLine), and use
-            // toString() since getMessage() text doesn't always mention 
error:)
-            throw new SQLException("Failure in starting embedded Drillbit: " + 
e, e);
+            Class.forName("org.eclipse.jetty.server.Handler");
+          } catch (final ClassNotFoundException e) {
+            throw new SQLNonTransientConnectionException(
+                "Running Drill in embedded mode using Drill's jdbc-all JDBC"
+                + " driver Jar file alone is not supported.",  e);
+          }
+
+          final DrillConfig dConfig = DrillConfig.create(info);
+          this.allocator = RootAllocatorFactory.newRoot(dConfig);
+          RemoteServiceSet set = GlobalServiceSetReference.SETS.get();
+          if (set == null) {
+            // We're embedded; start a local drill bit.
+            serviceSet = RemoteServiceSet.getLocalServiceSet();
+            set = serviceSet;
+            try {
+              bit = new Drillbit(dConfig, serviceSet);
+              bit.run();
+            } catch (final UserException e) {
+              throw new SQLException(
+                  "Failure in starting embedded Drillbit: " + e.getMessage(),
+                  e);
+            } catch (Exception e) {
+              // (Include cause exception's text in wrapping exception's text 
so
+              // it's more likely to get to user (e.g., via SQLLine), and use
+              // toString() since getMessage() text doesn't always mention 
error:)
+              throw new SQLException("Failure in starting embedded Drillbit: " 
+ e, e);
+            }
+          } else {
+            serviceSet = null;
+            bit = null;
           }
+
+          makeTmpSchemaLocationsUnique(bit.getContext().getStorage(), info);
+
+          this.client = new DrillClient(dConfig, set.getCoordinator());
+        } else if(config.isDirect()) {
+          final DrillConfig dConfig = DrillConfig.forClient();
+          this.allocator = RootAllocatorFactory.newRoot(dConfig);
+          this.client = new DrillClient(dConfig, true); // Get a direct 
connection
+          connect = config.getZookeeperConnectionString();
         } else {
-          serviceSet = null;
-          bit = null;
+          final DrillConfig dConfig = DrillConfig.forClient();
+          this.allocator = RootAllocatorFactory.newRoot(dConfig);
+          // TODO:  Check:  Why does new DrillClient() create another 
DrillConfig,
+          // with enableServerConfigs true, and cause scanning for function
+          // implementations (needed by a server, but not by a client-only
+          // process, right?)?  Probably pass dConfig to construction.
+          this.client = new DrillClient();
+          connect = config.getZookeeperConnectionString();
         }
-
-        makeTmpSchemaLocationsUnique(bit.getContext().getStorage(), info);
-
-        this.client = new DrillClient(dConfig, set.getCoordinator());
-      } else if(config.isDirect()) {
-        final DrillConfig dConfig = DrillConfig.forClient();
-        this.allocator = RootAllocatorFactory.newRoot(dConfig);
-        this.client = new DrillClient(dConfig, true); // Get a direct 
connection
-        connect = config.getZookeeperConnectionString();
-      } else {
-        final DrillConfig dConfig = DrillConfig.forClient();
-        this.allocator = RootAllocatorFactory.newRoot(dConfig);
-        // TODO:  Check:  Why does new DrillClient() create another 
DrillConfig,
-        // with enableServerConfigs true, and cause scanning for function
-        // implementations (needed by a server, but not by a client-only
-        // process, right?)?  Probably pass dConfig to construction.
-        this.client = new DrillClient();
-        connect = config.getZookeeperConnectionString();
+        this.client.setClientName("Apache Drill JDBC Driver");
+        this.client.connect(connect, info);
+      } catch (OutOfMemoryException e) {
+        throw new SQLNonTransientConnectionException("Failure creating root 
allocator", e);
+      } catch (InvalidConnectionInfoException e) {
+        throw new SQLNonTransientConnectionException("Invalid parameter in 
connection string: " + e.getMessage(), e);
+      } catch (RpcException e) {
+        // (Include cause exception's text in wrapping exception's text so
+        // it's more likely to get to user (e.g., via SQLLine), and use
+        // toString() since getMessage() text doesn't always mention error:)
+        throw new SQLNonTransientConnectionException("Failure in connecting to 
Drill: " + e, e);
+      } catch(SQLException e) {
+        throw e;
+      } catch (Exception e) {
+        throw new SQLException("Failure in creating DrillConnectionImpl: " + 
e, e);
       }
-      this.client.setClientName("Apache Drill JDBC Driver");
-      this.client.connect(connect, info);
-    } catch (OutOfMemoryException e) {
-      throw new SQLNonTransientConnectionException("Failure creating root 
allocator", e);
-    } catch (InvalidConnectionInfoException e) {
-      throw new SQLNonTransientConnectionException("Invalid parameter in 
connection string: " + e.getMessage(), e);
-    } catch (RpcException e) {
-      // (Include cause exception's text in wrapping exception's text so
-      // it's more likely to get to user (e.g., via SQLLine), and use
-      // toString() since getMessage() text doesn't always mention error:)
-      throw new SQLNonTransientConnectionException("Failure in connecting to 
Drill: " + e, e);
+    } catch (Throwable t) {
+      cleanup();
+      throw t;
     }
   }
 

Reply via email to