Repository: drill Updated Branches: refs/heads/master 05c42eae7 -> 7e564996f
DRILL-4768: Fix leaking hive meta store connection in Drill's hive metastore client call. - do not call reconnect if the connection is still alive and the error is caused by either UnknownTableException or access error. - call close() explicitly before reconnect() and check if client.close() will hit exception. - make DrillHiveMetaStoreClient closable. close apache/drill#543 Project: http://git-wip-us.apache.org/repos/asf/drill/repo Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/7e564996 Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/7e564996 Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/7e564996 Branch: refs/heads/master Commit: 7e564996f71330dd5acdae6d403841176eeb5c64 Parents: 05c42ea Author: Jinfeng Ni <[email protected]> Authored: Thu Jul 7 21:48:53 2016 -0700 Committer: Jinfeng Ni <[email protected]> Committed: Tue Jul 12 11:42:47 2016 -0700 ---------------------------------------------------------------------- .../store/hive/DrillHiveMetaStoreClient.java | 76 +++++++++++++++++--- .../store/hive/schema/HiveDatabaseSchema.java | 27 +++---- .../store/hive/schema/HiveSchemaFactory.java | 2 +- 3 files changed, 74 insertions(+), 31 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/drill/blob/7e564996/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/DrillHiveMetaStoreClient.java ---------------------------------------------------------------------- diff --git a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/DrillHiveMetaStoreClient.java b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/DrillHiveMetaStoreClient.java index df3e8a2..bbc1c70 100644 --- a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/DrillHiveMetaStoreClient.java +++ b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/DrillHiveMetaStoreClient.java @@ -29,9 +29,12 @@ import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.conf.HiveConf.ConfVars; import org.apache.hadoop.hive.metastore.HiveMetaStoreClient; import org.apache.hadoop.hive.metastore.IMetaStoreClient; +import org.apache.hadoop.hive.metastore.api.InvalidOperationException; import org.apache.hadoop.hive.metastore.api.MetaException; +import org.apache.hadoop.hive.metastore.api.NoSuchObjectException; import org.apache.hadoop.hive.metastore.api.Partition; import org.apache.hadoop.hive.metastore.api.Table; +import org.apache.hadoop.hive.metastore.api.UnknownDBException; import org.apache.hadoop.hive.metastore.api.UnknownTableException; import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAccessControlException; import org.apache.hadoop.hive.shims.Utils; @@ -127,9 +130,9 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient { * @return * @throws MetaException */ - public static DrillHiveMetaStoreClient createNonCloseableClientWithCaching(final HiveConf hiveConf) + public static DrillHiveMetaStoreClient createCloseableClientWithCaching(final HiveConf hiveConf) throws MetaException { - return new NonCloseableHiveClientWithCaching(hiveConf); + return new HiveClientWithCaching(hiveConf); } private DrillHiveMetaStoreClient(final HiveConf hiveConf) throws MetaException { @@ -197,8 +200,15 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient { protected static List<String> getDatabasesHelper(final IMetaStoreClient mClient) throws TException { try { return mClient.getAllDatabases(); + } catch (MetaException e) { + throw e; } catch (TException e) { - logger.warn("Failure while attempting to get hive databases", e); + logger.warn("Failure while attempting to get hive databases. Retries once.", e); + try { + mClient.close(); + } catch (Exception ex) { + logger.warn("Failure while attempting to close existing hive metastore connection. May leak connection.", ex); + } mClient.reconnect(); return mClient.getAllDatabases(); } @@ -209,8 +219,15 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient { throws TException { try { return mClient.getAllTables(dbName); + } catch (MetaException | UnknownDBException e) { + throw e; } catch (TException e) { - logger.warn("Failure while attempting to get hive tables", e); + logger.warn("Failure while attempting to get hive tables. Retries once.", e); + try { + mClient.close(); + } catch (Exception ex) { + logger.warn("Failure while attempting to close existing hive metastore connection. May leak connection.", ex); + } mClient.reconnect(); return mClient.getAllTables(dbName); } @@ -222,7 +239,15 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient { Table t = null; try { t = mClient.getTable(dbName, tableName); + } catch (MetaException | NoSuchObjectException e) { + throw e; } catch (TException e) { + logger.warn("Failure while attempting to get hive table. Retries once. ", e); + try { + mClient.close(); + } catch (Exception ex) { + logger.warn("Failure while attempting to close existing hive metastore connection. May leak connection.", ex); + } mClient.reconnect(); t = mClient.getTable(dbName, tableName); } @@ -234,7 +259,15 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient { List<Partition> partitions; try { partitions = mClient.listPartitions(dbName, tableName, (short) -1); + } catch (NoSuchObjectException | MetaException e) { + throw e; } catch (TException e) { + logger.warn("Failure while attempting to get hive partitions. Retries once. ", e); + try { + mClient.close(); + } catch (Exception ex) { + logger.warn("Failure while attempting to close existing hive metastore connection. May leak connection.", ex); + } mClient.reconnect(); partitions = mClient.listPartitions(dbName, tableName, (short) -1); } @@ -252,6 +285,32 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient { } /** + * Help method which gets hive tables for a given schema|DB name and a list of table names. + * Retries once if the first call fails with TExcption other than connection-lost problems. + * @param mClient + * @param schemaName + * @param tableNames + * @return list of hive table instances. + **/ + public static List<Table> getTableObjectsByNameHelper(final HiveMetaStoreClient mClient, final String schemaName, + final List<String> tableNames) throws TException { + try { + return mClient.getTableObjectsByName(schemaName, tableNames); + } catch (MetaException | InvalidOperationException | UnknownDBException e) { + throw e; + } catch (TException e) { + logger.warn("Failure while attempting to get tables by names. Retries once. ", e); + try { + mClient.close(); + } catch (Exception ex) { + logger.warn("Failure while attempting to close existing hive metastore connection. May leak connection.", ex); + } + mClient.reconnect(); + return mClient.getTableObjectsByName(schemaName, tableNames); + } + } + + /** * HiveMetaStoreClient to create and maintain (reconnection cases) connection to Hive metastore with given user * credentials and check authorization privileges if set. */ @@ -345,8 +404,8 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient { /** * HiveMetaStoreClient that provides a shared MetaStoreClient implementation with caching. */ - private static class NonCloseableHiveClientWithCaching extends DrillHiveMetaStoreClient { - private NonCloseableHiveClientWithCaching(final HiveConf hiveConf) throws MetaException { + private static class HiveClientWithCaching extends DrillHiveMetaStoreClient { + private HiveClientWithCaching(final HiveConf hiveConf) throws MetaException { super(hiveConf); } @@ -384,11 +443,6 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient { } } - @Override - public void close() { - // No-op. - } - } private class DatabaseLoader extends CacheLoader<String, List<String>> { http://git-wip-us.apache.org/repos/asf/drill/blob/7e564996/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveDatabaseSchema.java ---------------------------------------------------------------------- diff --git a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveDatabaseSchema.java b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveDatabaseSchema.java index ff61f8d..d07a69d 100644 --- a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveDatabaseSchema.java +++ b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveDatabaseSchema.java @@ -17,27 +17,24 @@ */ package org.apache.drill.exec.store.hive.schema; -import java.util.List; -import java.util.Set; - import com.google.common.collect.Lists; import com.google.common.collect.Sets; - import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rel.type.RelDataTypeFactory; import org.apache.calcite.schema.Schema; import org.apache.calcite.schema.Statistic; import org.apache.calcite.schema.Table; - import org.apache.commons.lang3.tuple.Pair; import org.apache.drill.exec.store.AbstractSchema; import org.apache.drill.exec.store.SchemaConfig; import org.apache.drill.exec.store.hive.DrillHiveMetaStoreClient; import org.apache.drill.exec.store.hive.HiveStoragePluginConfig; import org.apache.drill.exec.store.hive.schema.HiveSchemaFactory.HiveSchema; - import org.apache.thrift.TException; +import java.util.List; +import java.util.Set; + public class HiveDatabaseSchema extends AbstractSchema{ static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(HiveDatabaseSchema.class); @@ -85,19 +82,11 @@ public class HiveDatabaseSchema extends AbstractSchema{ final String schemaName = getName(); final List<Pair<String, ? extends Table>> tableNameToTable = Lists.newArrayList(); List<org.apache.hadoop.hive.metastore.api.Table> tables; - // Retries once if the first call to fetch the metadata fails - synchronized(mClient) { - try { - tables = mClient.getTableObjectsByName(schemaName, tableNames); - } catch(TException tException) { - try { - mClient.reconnect(); - tables = mClient.getTableObjectsByName(schemaName, tableNames); - } catch(Exception e) { - logger.warn("Exception occurred while trying to read tables from {}: {}", schemaName, e.getCause()); - return tableNameToTable; - } - } + try { + tables = DrillHiveMetaStoreClient.getTableObjectsByNameHelper(mClient, schemaName, tableNames); + } catch (TException e) { + logger.warn("Exception occurred while trying to list tables by names from {}: {}", schemaName, e.getCause()); + return tableNameToTable; } for(final org.apache.hadoop.hive.metastore.api.Table table : tables) { http://git-wip-us.apache.org/repos/asf/drill/blob/7e564996/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java ---------------------------------------------------------------------- diff --git a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java index 5eae544..9cfaeae 100644 --- a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java +++ b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java @@ -73,7 +73,7 @@ public class HiveSchemaFactory implements SchemaFactory { try { processUserMetastoreClient = - DrillHiveMetaStoreClient.createNonCloseableClientWithCaching(hiveConf); + DrillHiveMetaStoreClient.createCloseableClientWithCaching(hiveConf); } catch (MetaException e) { throw new ExecutionSetupException("Failure setting up Hive metastore client.", e); }
