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 <[email protected]> Authored: Wed Jan 17 14:35:33 2018 +0530 Committer: Aman Sinha <[email protected]> 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; } }
