diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseClassTestRule.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseClassTestRule.java index 00374c115abb..0880ad08ba81 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseClassTestRule.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseClassTestRule.java @@ -1,4 +1,4 @@ -/** +/* * 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 @@ -17,10 +17,16 @@ */ package org.apache.hadoop.hbase; +import edu.umd.cs.findbugs.annotations.NonNull; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.Arrays; +import java.util.Collection; import java.util.Collections; +import java.util.List; import java.util.Set; import java.util.concurrent.TimeUnit; - import org.apache.hadoop.hbase.testclassification.IntegrationTests; import org.apache.hadoop.hbase.testclassification.LargeTests; import org.apache.hadoop.hbase.testclassification.MediumTests; @@ -30,8 +36,14 @@ import org.junit.rules.TestRule; import org.junit.rules.Timeout; import org.junit.runner.Description; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; import org.junit.runners.model.Statement; - +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; +import org.apache.hbase.thirdparty.com.google.common.collect.Iterables; import org.apache.hbase.thirdparty.com.google.common.collect.Sets; /** @@ -43,9 +55,13 @@ */ @InterfaceAudience.Private public final class HBaseClassTestRule implements TestRule { + private static final Logger LOG = LoggerFactory.getLogger(HBaseClassTestRule.class); public static final Set> UNIT_TEST_CLASSES = Collections.unmodifiableSet( Sets.> newHashSet(SmallTests.class, MediumTests.class, LargeTests.class)); + // Each unit test has this timeout. + private static long PER_UNIT_TEST_TIMEOUT_MINS = 13; + private final Class clazz; private final Timeout timeout; @@ -65,13 +81,16 @@ public Class getClazz() { private static long getTimeoutInSeconds(Class clazz) { Category[] categories = clazz.getAnnotationsByType(Category.class); - + // Starting JUnit 4.13, it appears that the timeout is applied across all the parameterized + // runs. So the timeout is multiplied by number of parameterized runs. + int numParams = getNumParameters(clazz); // @Category is not repeatable -- it is only possible to get an array of length zero or one. if (categories.length == 1) { for (Class c : categories[0].value()) { if (UNIT_TEST_CLASSES.contains(c)) { - // All tests have a 13 minutes timeout. - return TimeUnit.MINUTES.toSeconds(13); + long timeout = numParams * PER_UNIT_TEST_TIMEOUT_MINS; + LOG.info("Test {} timeout: {} mins", clazz, timeout); + return TimeUnit.MINUTES.toSeconds(timeout); } if (c == IntegrationTests.class) { return TimeUnit.MINUTES.toSeconds(Long.MAX_VALUE); @@ -82,6 +101,59 @@ private static long getTimeoutInSeconds(Class clazz) { clazz.getName() + " does not have SmallTests/MediumTests/LargeTests in @Category"); } + /** + * @param clazz Test class that is running. + * @return the number of parameters for this given test class. If the test is not parameterized or + * if there is any issue determining the number of parameters, returns 1. + */ + @VisibleForTesting + static int getNumParameters(Class clazz) { + RunWith[] runWiths = clazz.getAnnotationsByType(RunWith.class); + boolean testParameterized = runWiths != null && Arrays.stream(runWiths).anyMatch( + (r) -> r.value().equals(Parameterized.class)); + if (!testParameterized) { + return 1; + } + for (Method method : clazz.getMethods()) { + if (!isParametersMethod(method)) { + continue; + } + // Found the parameters method. Figure out the number of parameters. + Object parameters; + try { + parameters = method.invoke(clazz); + } catch (IllegalAccessException | InvocationTargetException e) { + LOG.warn("Error invoking parameters method {} in test class {}", + method.getName(), clazz, e); + continue; + } + if (parameters instanceof List) { + return ((List) parameters).size(); + } else if (parameters instanceof Collection) { + return ((Collection) parameters).size(); + } else if (parameters instanceof Iterable) { + return Iterables.size((Iterable) parameters); + } else if (parameters instanceof Object[]) { + return ((Object[]) parameters).length; + } + } + LOG.warn("Unable to determine parameters size. Returning the default of 1."); + return 1; + } + + /** + * Helper method that checks if the input method is a valid JUnit @Parameters method. + * @param method Input method. + * @return true if the method is a valid JUnit parameters method, false otherwise. + */ + private static boolean isParametersMethod(@NonNull Method method) { + // A valid parameters method is public static and with @Parameters annotation. + boolean methodPublicStatic = Modifier.isPublic(method.getModifiers()) && + Modifier.isStatic(method.getModifiers()); + Parameters[] params = method.getAnnotationsByType(Parameters.class); + return methodPublicStatic && (params != null && params.length > 0); + } + public static HBaseClassTestRule forClass(Class clazz) { return new HBaseClassTestRule(clazz, Timeout.builder().withLookingForStuckThread(true) .withTimeout(getTimeoutInSeconds(clazz), TimeUnit.SECONDS).build()); diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestHBaseClassTestRule.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/TestHBaseClassTestRule.java new file mode 100644 index 000000000000..78853e6aed76 --- /dev/null +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/TestHBaseClassTestRule.java @@ -0,0 +1,145 @@ +/* + * 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.hbase; + +import static junit.framework.TestCase.assertEquals; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; +import org.apache.hbase.thirdparty.com.google.common.collect.Iterables; + +/** + * Tests HBaseClassTestRule. + */ +@Category(SmallTests.class) +public class TestHBaseClassTestRule { + + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = HBaseClassTestRule.forClass( + TestHBaseClassTestRule.class); + + // Test input classes of various kinds. + private static class NonParameterizedClass { + void dummy() { + } + int dummy(int a) { + return 0; + } + } + + @RunWith(Parameterized.class) + private static class ParameterizedClassWithNoParametersMethod { + void dummy() { + } + } + + @RunWith(Parameterized.class) + private static class InValidParameterizedClass { + // Not valid because parameters method is private. + @Parameters + private static List parameters() { + return Arrays.asList(1, 2, 3, 4); + } + int dummy(int a) { + return 0; + } + } + + @RunWith(Parameterized.class) + private static class ValidParameterizedClass1 { + @Parameters + public static List parameters() { + return Arrays.asList(1, 2, 3, 4, 5); + } + int dummy(int a) { + return 0; + } + } + + @RunWith(Parameterized.class) + private static class ValidParameterizedClass2 { + @Parameters + public static Object[] parameters() { + return new Integer[] {1, 2, 3, 4, 5, 6}; + } + } + + @RunWith(Parameterized.class) + private static class ValidParameterizedClass3 { + @Parameters + public static Iterable parameters() { + return Arrays.asList(1, 2, 3, 4, 5, 6, 7); + } + } + + @RunWith(Parameterized.class) + private static class ValidParameterizedClass4 { + @Parameters + public static Collection parameters() { + return Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9); + } + } + + + @RunWith(Parameterized.class) + private static class ExtendedParameterizedClass1 extends ValidParameterizedClass1 { + // Should be inferred from the parent class. + int dummy2(int a) { + return 0; + } + } + + @RunWith(Parameterized.class) + private static class ExtendedParameterizedClass2 extends ValidParameterizedClass1 { + // Should override the parent parameters class. + @Parameters + public static List parameters() { + return Arrays.asList(1, 2, 3); + } + } + + @Test + public void testNumParameters() { + // Invalid cases, expected to return 1. + assertEquals(HBaseClassTestRule.getNumParameters(NonParameterizedClass.class), 1); + assertEquals(HBaseClassTestRule.getNumParameters( + ParameterizedClassWithNoParametersMethod.class), 1); + assertEquals(HBaseClassTestRule.getNumParameters(InValidParameterizedClass.class), 1); + // Valid parameterized classes. + assertEquals(HBaseClassTestRule.getNumParameters(ValidParameterizedClass1.class), + ValidParameterizedClass1.parameters().size()); + assertEquals(HBaseClassTestRule.getNumParameters(ValidParameterizedClass2.class), + ValidParameterizedClass2.parameters().length); + assertEquals(HBaseClassTestRule.getNumParameters(ValidParameterizedClass3.class), + Iterables.size(ValidParameterizedClass3.parameters())); + assertEquals(HBaseClassTestRule.getNumParameters(ValidParameterizedClass4.class), + ValidParameterizedClass4.parameters().size()); + // Testing inheritance. + assertEquals(HBaseClassTestRule.getNumParameters(ExtendedParameterizedClass1.class), + ValidParameterizedClass1.parameters().size()); + assertEquals(HBaseClassTestRule.getNumParameters(ExtendedParameterizedClass2.class), + ExtendedParameterizedClass2.parameters().size()); + } +} diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index d8f71a9e8f5c..93ae12e27d29 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -20,7 +20,6 @@ import static org.apache.hadoop.hbase.HConstants.DEFAULT_HBASE_SPLIT_COORDINATED_BY_ZK; import static org.apache.hadoop.hbase.HConstants.HBASE_MASTER_LOGCLEANER_PLUGINS; import static org.apache.hadoop.hbase.HConstants.HBASE_SPLIT_WAL_COORDINATED_BY_ZK; - import com.google.protobuf.Descriptors; import com.google.protobuf.Service; import java.io.IOException; @@ -56,8 +55,10 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.ChoreService; import org.apache.hadoop.hbase.ClusterId; @@ -219,11 +220,9 @@ import org.eclipse.jetty.webapp.WebAppContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; - import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; import org.apache.hbase.thirdparty.com.google.common.collect.Lists; import org.apache.hbase.thirdparty.com.google.common.collect.Maps; - import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.shaded.protobuf.RequestConverter; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.GetRegionInfoResponse.CompactionState; @@ -913,8 +912,15 @@ private void finishActiveMasterInitialization(MonitoredTask status) throws IOExc // hbck1s against an hbase2 cluster; it could do damage. To skip this behavior, set // hbase.write.hbck1.lock.file to false. if (this.conf.getBoolean("hbase.write.hbck1.lock.file", true)) { - HBaseFsck.checkAndMarkRunningHbck(this.conf, - HBaseFsck.createLockRetryCounterFactory(this.conf).create()); + Pair result = null; + try { + result = HBaseFsck.checkAndMarkRunningHbck(this.conf, + HBaseFsck.createLockRetryCounterFactory(this.conf).create()); + } finally { + if (result != null) { + IOUtils.closeQuietly(result.getSecond()); + } + } } status.setStatus("Initialize ServerManager and schedule SCP for crash servers");