commit bf37621c9a81b49223b3d5452ed559bf7018cecf Author: Daniel Dai Date: Fri Dec 18 12:17:23 2015 -0800 HIVE-9642: Hive metastore client retries don't happen consistently for all api calls diff --git a/hcatalog/core/src/test/java/org/apache/hive/hcatalog/cli/TestSemanticAnalysis.java b/hcatalog/core/src/test/java/org/apache/hive/hcatalog/cli/TestSemanticAnalysis.java index cf15ff2..4f53fcb 100644 --- a/hcatalog/core/src/test/java/org/apache/hive/hcatalog/cli/TestSemanticAnalysis.java +++ b/hcatalog/core/src/test/java/org/apache/hive/hcatalog/cli/TestSemanticAnalysis.java @@ -126,7 +126,7 @@ public void testUsNonExistentDB() throws CommandNeedRetryException { } @Test - public void testDatabaseOperations() throws MetaException, CommandNeedRetryException { + public void testDatabaseOperations() throws CommandNeedRetryException, TException { List dbs = client.getAllDatabases(); String testDb1 = "testdatabaseoperatons1"; diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java index 178796d..f054486 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java @@ -1068,24 +1068,14 @@ public boolean dropType(String type) throws NoSuchObjectException, MetaException /** {@inheritDoc} */ @Override public List getDatabases(String databasePattern) - throws MetaException { - try { + throws MetaException, TException { return filterHook.filterDatabases(client.get_databases(databasePattern)); - } catch (Exception e) { - MetaStoreUtils.logAndThrowMetaException(e); - } - return null; } /** {@inheritDoc} */ @Override - public List getAllDatabases() throws MetaException { - try { - return filterHook.filterDatabases(client.get_all_databases()); - } catch (Exception e) { - MetaStoreUtils.logAndThrowMetaException(e); - } - return null; + public List getAllDatabases() throws MetaException, TException { + return filterHook.filterDatabases(client.get_all_databases()); } /** @@ -1309,24 +1299,14 @@ public Type getType(String name) throws NoSuchObjectException, MetaException, TE /** {@inheritDoc} */ @Override - public List getTables(String dbname, String tablePattern) throws MetaException { - try { - return filterHook.filterTableNames(dbname, client.get_tables(dbname, tablePattern)); - } catch (Exception e) { - MetaStoreUtils.logAndThrowMetaException(e); - } - return null; + public List getTables(String dbname, String tablePattern) throws MetaException, TException { + return filterHook.filterTableNames(dbname, client.get_tables(dbname, tablePattern)); } @Override public List getTableMeta(String dbPatterns, String tablePatterns, List tableTypes) - throws MetaException { - try { - return filterNames(client.get_table_meta(dbPatterns, tablePatterns, tableTypes)); - } catch (Exception e) { - MetaStoreUtils.logAndThrowMetaException(e); - } - return null; + throws MetaException, TException { + return filterNames(client.get_table_meta(dbPatterns, tablePatterns, tableTypes)); } private List filterNames(List metas) throws MetaException { @@ -1351,13 +1331,8 @@ public Type getType(String name) throws NoSuchObjectException, MetaException, TE /** {@inheritDoc} */ @Override - public List getAllTables(String dbname) throws MetaException { - try { - return filterHook.filterTableNames(dbname, client.get_all_tables(dbname)); - } catch (Exception e) { - MetaStoreUtils.logAndThrowMetaException(e); - } - return null; + public List getAllTables(String dbname) throws MetaException, TException { + return filterHook.filterTableNames(dbname, client.get_all_tables(dbname)); } @Override diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingMetaStoreClient.java b/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingMetaStoreClient.java index 2b05837..3f5aea2 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingMetaStoreClient.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingMetaStoreClient.java @@ -82,8 +82,26 @@ protected RetryingMetaStoreClient(HiveConf hiveConf, Class[] constructorArgTy String msUri = hiveConf.getVar(HiveConf.ConfVars.METASTOREURIS); localMetaStore = (msUri == null) || msUri.trim().isEmpty(); - reloginExpiringKeytabUser(); - this.base = (IMetaStoreClient) MetaStoreUtils.newInstance(msClientClass, constructorArgTypes, constructorArgs); + int retriesMade = 0; + IMetaStoreClient metaStoreClient = null; + while (true) { + try { + reloginExpiringKeytabUser(); + metaStoreClient = (IMetaStoreClient) MetaStoreUtils.newInstance(msClientClass, constructorArgTypes, constructorArgs); + break; + } catch (Exception e) { + if (retriesMade >= retryLimit) { + throw e; + } + retriesMade++; + LOG.warn("Construct for " + msClientClass + " failed, retrying.", e); + try { + Thread.sleep(retryDelaySeconds * 1000); + } catch (InterruptedException ie) { + } + } + } + this.base = metaStoreClient; } public static IMetaStoreClient getProxy(HiveConf hiveConf) throws MetaException { @@ -144,7 +162,7 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl while (true) { try { reloginExpiringKeytabUser(); - if (retriesMade > 0 || hasConnectionLifeTimeReached(method)) { + if (!localMetaStore && (retriesMade > 0 || hasConnectionLifeTimeReached(method))) { base.reconnect(); lastConnectionTime = System.currentTimeMillis(); } @@ -180,6 +198,8 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl } else if ((t instanceof MetaException) && t.getMessage().matches( "(?s).*(JDO[a-zA-Z]*|TProtocol|TTransport)Exception.*")) { caughtException = (MetaException)t; + } else if (t instanceof TException) { + caughtException = (TException)t; } else { throw t; } diff --git a/metastore/src/test/org/apache/hadoop/hive/metastore/TestRetryingHiveMetaStoreClient.java b/metastore/src/test/org/apache/hadoop/hive/metastore/TestRetryingHiveMetaStoreClient.java new file mode 100644 index 0000000..7769707 --- /dev/null +++ b/metastore/src/test/org/apache/hadoop/hive/metastore/TestRetryingHiveMetaStoreClient.java @@ -0,0 +1,197 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hive.metastore; + +import java.util.List; + +import org.apache.hadoop.hive.conf.HiveConf; +import org.apache.hadoop.hive.metastore.TestHiveMetaStorePartitionSpecs.NoExitSecurityManager; +import org.apache.hadoop.hive.metastore.api.MetaException; +import org.apache.hadoop.hive.metastore.api.TableMeta; +import org.apache.thrift.TException; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class TestRetryingHiveMetaStoreClient { + private static final String msPort = "20102"; + private static HiveConf hiveConf; + private static SecurityManager securityManager; + private static final Logger LOG = LoggerFactory.getLogger(TestRetryingHiveMetaStoreClient.class); + + static class RetryingTestHiveMetaStoreClient extends HiveMetaStoreClient implements IMetaStoreClient { + static int constructRetryingCount = 0; + static int getDatabasesRetryingCount = 0; + static int getAllDatabasesRetryingCount = 0; + static int getTablesRetryingCount = 0; + static int getTableMetaRetryingCount = 0; + static int getAllTablesRetryingCount = 0; + + public RetryingTestHiveMetaStoreClient(HiveConf conf) throws MetaException { + super(conf); + if (constructRetryingCount <= 1) { + constructRetryingCount++; + throw new RuntimeException("Intentionally to test RetryingTestHiveMetaStoreClient"); + } + } + + public static void resetAllCounts() { + constructRetryingCount = 0; + getDatabasesRetryingCount = 0; + getAllDatabasesRetryingCount = 0; + getTablesRetryingCount = 0; + getTableMetaRetryingCount = 0; + getAllTablesRetryingCount = 0; + } + + @Override + public List getDatabases(String databasePattern) throws MetaException, TException { + if (getDatabasesRetryingCount <= 1) { + getDatabasesRetryingCount++; + throw new TException("Intentionally to test RetryingTestHiveMetaStoreClient"); + } + return super.getDatabases(".*"); + } + + @Override + public List getAllDatabases() throws MetaException, TException { + if (getAllDatabasesRetryingCount <= 1) { + getAllDatabasesRetryingCount++; + throw new TException("Intentionally to test RetryingTestHiveMetaStoreClient"); + } + return super.getAllDatabases(); + } + + @Override + public List getTables(String dbname, String tablePattern) throws MetaException, TException { + if (getTablesRetryingCount <= 1) { + getTablesRetryingCount++; + throw new TException("Intentionally to test RetryingTestHiveMetaStoreClient"); + } + return super.getTables(dbname, tablePattern); + } + + @Override + public List getTableMeta(String dbPatterns, String tablePatterns, List tableTypes) throws MetaException, TException { + if (getTableMetaRetryingCount <= 1) { + getTableMetaRetryingCount++; + throw new TException("Intentionally to test RetryingTestHiveMetaStoreClient"); + } + return super.getTableMeta(dbPatterns, tablePatterns, tableTypes); + } + + @Override + public List getAllTables(String dbname) throws MetaException, TException { + if (getAllTablesRetryingCount <= 1) { + getAllTablesRetryingCount++; + throw new TException("Intentionally to test RetryingTestHiveMetaStoreClient"); + } + return super.getAllTables(dbname); + } + + public static int getConstructRetryingCount() { + return constructRetryingCount; + } + + public static int getGetDatabasesRetryingCount() { + return getDatabasesRetryingCount; + } + + public static int getAllDatabasesRetryingCount() { + return getAllDatabasesRetryingCount; + } + + public static int getTablesRetryingCount() { + return getTablesRetryingCount; + } + + public static int getTableMetaRetryingCount() { + return getTableMetaRetryingCount; + } + + public static int getAllTablesRetryingCount() { + return getAllTablesRetryingCount; + } + } + + private static class RunMS implements Runnable { + @Override + public void run() { + try { + HiveMetaStore.main(new String[]{"-v", "-p", msPort, "--hiveconf", + "hive.metastore.expression.proxy=" + MockPartitionExpressionForMetastore.class.getCanonicalName()}); + } catch (Throwable t) { + LOG.error("Exiting. Got exception from metastore: ", t); + } + } + } + + @BeforeClass + public static void startMetaStoreServer() throws Exception { + + Thread t = new Thread(new RunMS()); + t.start(); + Thread.sleep(5000); + + securityManager = System.getSecurityManager(); + System.setSecurityManager(new NoExitSecurityManager()); + hiveConf = new HiveConf(TestRetryingHiveMetaStoreClient.class); + hiveConf.setVar(HiveConf.ConfVars.METASTOREURIS, "thrift://localhost:" + + msPort); + hiveConf.setIntVar(HiveConf.ConfVars.METASTORETHRIFTFAILURERETRIES, 3); + hiveConf.set(HiveConf.ConfVars.PREEXECHOOKS.varname, ""); + hiveConf.set(HiveConf.ConfVars.POSTEXECHOOKS.varname, ""); + hiveConf.set(HiveConf.ConfVars.HIVE_SUPPORT_CONCURRENCY.varname, + "false"); + System.setProperty(HiveConf.ConfVars.PREEXECHOOKS.varname, " "); + System.setProperty(HiveConf.ConfVars.POSTEXECHOOKS.varname, " "); + } + + @AfterClass + public static void tearDown() throws Exception { + LOG.info("Shutting down metastore."); + System.setSecurityManager(securityManager); + } + + @Test + public void testRetryingMetaStoreClient() throws Exception { + RetryingTestHiveMetaStoreClient.resetAllCounts(); + IMetaStoreClient retryingHiveMetaStoreClient = + RetryingMetaStoreClient.getProxy(hiveConf, + new Class[]{HiveConf.class}, new Object[]{hiveConf}, null, RetryingTestHiveMetaStoreClient.class.getName()); + Assert.assertEquals(RetryingTestHiveMetaStoreClient.getConstructRetryingCount(), 2); + + retryingHiveMetaStoreClient.getDatabases("*"); + Assert.assertEquals(RetryingTestHiveMetaStoreClient.getGetDatabasesRetryingCount(), 2); + + retryingHiveMetaStoreClient.getAllDatabases(); + Assert.assertEquals(RetryingTestHiveMetaStoreClient.getAllDatabasesRetryingCount(), 2); + + retryingHiveMetaStoreClient.getTables("default", "*"); + Assert.assertEquals(RetryingTestHiveMetaStoreClient.getTablesRetryingCount(), 2); + + retryingHiveMetaStoreClient.getTableMeta("default", "*", null); + Assert.assertEquals(RetryingTestHiveMetaStoreClient.getTableMetaRetryingCount(), 2); + + retryingHiveMetaStoreClient.getAllTables("default"); + Assert.assertEquals(RetryingTestHiveMetaStoreClient.getAllTablesRetryingCount(), 2); + } +} diff --git a/ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java b/ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java index 581a919..b26ea53 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java @@ -125,7 +125,7 @@ protected void drop_table_with_environment_context(String dbname, String name, } @Override - public List getAllTables(String dbName) throws MetaException { + public List getAllTables(String dbName) throws MetaException, TException { List tableNames = super.getAllTables(dbName); // May need to merge with list of temp tables @@ -147,7 +147,7 @@ protected void drop_table_with_environment_context(String dbname, String name, } @Override - public List getTables(String dbName, String tablePattern) throws MetaException { + public List getTables(String dbName, String tablePattern) throws MetaException, TException { List tableNames = super.getTables(dbName, tablePattern); // May need to merge with list of temp tables @@ -177,7 +177,7 @@ protected void drop_table_with_environment_context(String dbname, String name, @Override public List getTableMeta(String dbPatterns, String tablePatterns, List tableTypes) - throws MetaException { + throws MetaException, TException { List tableMetas = super.getTableMeta(dbPatterns, tablePatterns, tableTypes); Map> tmpTables = getTempTables(); if (tmpTables.isEmpty()) {