This is an automated email from the ASF dual-hosted git repository.
bharathv pushed a commit to branch HBASE-18095/client-locate-meta-no-zookeeper
in repository https://gitbox.apache.org/repos/asf/hbase.git
commit 26e9f5a2725277d78d564f04c0d47cc52934f894
Author: Bharath Vissapragada <bhara...@apache.org>
AuthorDate: Mon Jan 27 12:37:02 2020 -0800
HBASE-23731: De-flake TestFromClientSide (#1091)
There were a couple of issues.
- There was a leak of a file descriptor for hbck lock file. This
was contributing to all the "ConnectionRefused" stack traces since
it was trying to renew lease for an already expired mini dfs cluster.
This issue was there for a while, just that we noticed it now.
- After upgrade to JUnit 4.13, it looks like the behavior for test
timeouts has changed. Earlier the timeout seems to have applied for
each parameterized run, but now it looks like it is applied across
all the runs.
This patch fixes both the issues.
Signed-off-by: Stack <st...@apache.org>
Signed-off-by: Jan Hentschel <jan.hentsc...@ultratendency.com>
---
.../apache/hadoop/hbase/HBaseClassTestRule.java | 84 +++++++++++-
.../hadoop/hbase/TestHBaseClassTestRule.java | 145 +++++++++++++++++++++
.../org/apache/hadoop/hbase/master/HMaster.java | 16 ++-
3 files changed, 234 insertions(+), 11 deletions(-)
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 00374c1..0880ad0 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.experimental.categories.Category;
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 @@ import
org.apache.hbase.thirdparty.com.google.common.collect.Sets;
*/
@InterfaceAudience.Private
public final class HBaseClassTestRule implements TestRule {
+ private static final Logger LOG =
LoggerFactory.getLogger(HBaseClassTestRule.class);
public static final Set<Class<?>> UNIT_TEST_CLASSES =
Collections.unmodifiableSet(
Sets.<Class<?>> 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 final class HBaseClassTestRule implements TestRule {
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 @@ public final class HBaseClassTestRule implements TestRule {
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 0000000..78853e6
--- /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<Object> 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<Object> 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<Integer> parameters() {
+ return Arrays.asList(1, 2, 3, 4, 5, 6, 7);
+ }
+ }
+
+ @RunWith(Parameterized.class)
+ private static class ValidParameterizedClass4 {
+ @Parameters
+ public static Collection<Integer> 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<Object> 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 def82b4..0b6f994 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 @@ package org.apache.hadoop.hbase.master;
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.ServletException;
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.servlet.ServletHolder;
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;
@@ -922,8 +921,15 @@ public class HMaster extends HRegionServer implements
MasterServices {
// 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<Path, FSDataOutputStream> 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");
