commit 8fe355d7d9f9ab55b2b0c7e0ff5e3809a6ccfcc4 Author: Vihang Karajgaonkar Date: Wed Jan 11 10:45:04 2017 -0800 HIVE-15569 : failures in RetryingHMSHandler. do not get retried diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java b/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java index e46b50d5d02f87ade2a894546346ee2c8cd79916..f19ff6c312a9f4bc8fd436e2ae29cd523ee0fe3b 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java @@ -68,14 +68,22 @@ private RetryingHMSHandler(HiveConf hiveConf, IHMSHandler baseHandler, boolean l baseHandler.setConf(hiveConf); // tests expect configuration changes applied directly to metastore } activeConf = baseHandler.getConf(); - // This has to be called before initializing the instance of HMSHandler // Using the hook on startup ensures that the hook always has priority // over settings in *.xml. The thread local conf needs to be used because at this point // it has already been initialized using hiveConf. MetaStoreInit.updateConnectionURL(hiveConf, getActiveConf(), null, metaStoreInitData); - - baseHandler.init(); + try { + //invoking init method of baseHandler this way since it adds the retry logic + //in case of transient failures in init method + invoke(baseHandler, baseHandler.getClass().getDeclaredMethod("init", (Class[]) null), + null); + } catch (Throwable e) { + LOG.error("HMSHandler Fatal error: " + ExceptionUtils.getStackTrace(e)); + MetaException me = new MetaException(e.getMessage()); + me.initCause(e); + throw me; + } } public static IHMSHandler getProxy(HiveConf hiveConf, IHMSHandler baseHandler, boolean local) diff --git a/metastore/src/test/org/apache/hadoop/hive/metastore/TestRetriesInRetryingHMSHandler.java b/metastore/src/test/org/apache/hadoop/hive/metastore/TestRetriesInRetryingHMSHandler.java new file mode 100644 index 0000000000000000000000000000000000000000..d77cc274bd42f376a281d49a95cfdf6635d14a19 --- /dev/null +++ b/metastore/src/test/org/apache/hadoop/hive/metastore/TestRetriesInRetryingHMSHandler.java @@ -0,0 +1,109 @@ +/** + * 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.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.util.concurrent.TimeUnit; + +import javax.jdo.JDOException; + +import org.apache.hadoop.hive.conf.HiveConf; +import org.apache.hadoop.hive.metastore.api.MetaException; +import org.junit.BeforeClass; +import org.junit.Test; +import org.mockito.Mockito; + +public class TestRetriesInRetryingHMSHandler { + + private static HiveConf hiveConf; + private static final int RETRY_ATTEMPTS = 3; + + @BeforeClass + public static void setup() throws IOException { + hiveConf = new HiveConf(); + int port = MetaStoreUtils.findFreePort(); + hiveConf.setVar(HiveConf.ConfVars.METASTOREURIS, "thrift://localhost:" + port); + hiveConf.setIntVar(HiveConf.ConfVars.METASTORETHRIFTCONNECTIONRETRIES, 3); + hiveConf.setIntVar(HiveConf.ConfVars.HMSHANDLERATTEMPTS, RETRY_ATTEMPTS); + hiveConf.setTimeVar(HiveConf.ConfVars.HMSHANDLERINTERVAL, 10, TimeUnit.MILLISECONDS); + hiveConf.setBoolVar(HiveConf.ConfVars.HMSHANDLERFORCERELOADCONF, false); + } + + /* + * If the init method of HMSHandler throws exception for the first time + * while creating RetryingHMSHandler it should be retried + */ + @Test + public void testRetryInit() throws MetaException { + IHMSHandler mockBaseHandler = Mockito.mock(HiveMetaStore.HMSHandler.class); + Mockito.when(mockBaseHandler.getConf()).thenReturn(hiveConf); + Mockito + .doThrow(JDOException.class) + .doNothing() + .when(mockBaseHandler).init(); + RetryingHMSHandler.getProxy(hiveConf, mockBaseHandler, false); + Mockito.verify(mockBaseHandler, Mockito.times(2)).init(); + } + + /* + * init method in HMSHandler should not be retried if there are no exceptions + */ + @Test + public void testNoRetryInit() throws MetaException { + IHMSHandler mockBaseHandler = Mockito.mock(HiveMetaStore.HMSHandler.class); + Mockito.when(mockBaseHandler.getConf()).thenReturn(hiveConf); + Mockito.doNothing().when(mockBaseHandler).init(); + RetryingHMSHandler.getProxy(hiveConf, mockBaseHandler, false); + Mockito.verify(mockBaseHandler, Mockito.times(1)).init(); + } + + /* + * If the init method in HMSHandler throws exception all the times it should be retried until + * HiveConf.ConfVars.HMSHANDLERATTEMPTS is reached before giving up + */ + @Test(expected = MetaException.class) + public void testRetriesLimit() throws MetaException { + IHMSHandler mockBaseHandler = Mockito.mock(HiveMetaStore.HMSHandler.class); + Mockito.when(mockBaseHandler.getConf()).thenReturn(hiveConf); + Mockito.doThrow(JDOException.class).when(mockBaseHandler).init(); + RetryingHMSHandler.getProxy(hiveConf, mockBaseHandler, false); + Mockito.verify(mockBaseHandler, Mockito.times(RETRY_ATTEMPTS)).init(); + } + + /* + * Test retries when InvocationException wrapped in MetaException wrapped in JDOException + * is thrown + */ + @Test + public void testWrappedMetaExceptionRetry() throws MetaException { + IHMSHandler mockBaseHandler = Mockito.mock(HiveMetaStore.HMSHandler.class); + Mockito.when(mockBaseHandler.getConf()).thenReturn(hiveConf); + //JDOException wrapped in MetaException wrapped in InvocationException + MetaException me = new MetaException("Dummy exception"); + me.initCause(new JDOException()); + InvocationTargetException ex = new InvocationTargetException(me); + Mockito + .doThrow(me) + .doNothing() + .when(mockBaseHandler).init(); + RetryingHMSHandler.getProxy(hiveConf, mockBaseHandler, false); + Mockito.verify(mockBaseHandler, Mockito.times(2)).init(); + } +}