Skip to content

Commit 7f1ea6a

Browse files
Apache9Apache9
authored
HBASE-29966 Fix race in FlushRegionProcedure and rewrite TestAcidGuarantees (#7886)
Signed-off-by: Hui Ruan <huiruan@apache.org> Reviewed-by: Liu Xiao <liuxiao2103@qq.com>
1 parent 418a3a6 commit 7f1ea6a

File tree

9 files changed

+173
-97
lines changed

9 files changed

+173
-97
lines changed

‎hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseParameterizedTemplateProvider.java‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import org.junit.jupiter.api.extension.TestTemplateInvocationContext;
2828
import org.junit.jupiter.api.extension.TestTemplateInvocationContextProvider;
2929
import org.junit.jupiter.params.provider.Arguments;
30+
import org.slf4j.Logger;
31+
import org.slf4j.LoggerFactory;
3032

3133
/**
3234
* The entry point class for supporting JUnit4 like Parameterized test, where we can use constructor
@@ -48,6 +50,9 @@
4850
@InterfaceAudience.Private
4951
public class HBaseParameterizedTemplateProvider implements TestTemplateInvocationContextProvider {
5052

53+
private static final Logger LOG =
54+
LoggerFactory.getLogger(HBaseParameterizedTemplateProvider.class);
55+
5156
private static final String PARAMETERS_METHOD_NAME = "parameters";
5257

5358
@Override
@@ -56,19 +61,31 @@ public boolean supportsTestTemplate(ExtensionContext context) {
5661
.map(c -> c.isAnnotationPresent(HBaseParameterizedTestTemplate.class)).orElse(false);
5762
}
5863

64+
private Method findParametersMethod(final Class<?> testClass) {
65+
Class<?> clazz = testClass;
66+
for (;;) {
67+
try {
68+
return clazz.getDeclaredMethod(PARAMETERS_METHOD_NAME);
69+
} catch (NoSuchMethodException e) {
70+
Class<?> parentClass = clazz.getSuperclass();
71+
LOG.debug("Can not find method {} in class {}, try its parent class {}",
72+
PARAMETERS_METHOD_NAME, clazz, parentClass);
73+
if (parentClass == null) {
74+
throw new ExtensionConfigurationException("Can not find a static "
75+
+ PARAMETERS_METHOD_NAME + " method in class " + testClass + "or its super classes");
76+
}
77+
clazz = parentClass;
78+
}
79+
}
80+
81+
}
82+
5983
@Override
6084
public Stream<TestTemplateInvocationContext>
6185
provideTestTemplateInvocationContexts(ExtensionContext context) {
6286
Class<?> testClass = context.getRequiredTestClass();
6387
// get parameters
64-
Method method;
65-
try {
66-
method = testClass.getDeclaredMethod(PARAMETERS_METHOD_NAME);
67-
} catch (NoSuchMethodException e) {
68-
throw new ExtensionConfigurationException(
69-
"Test class must declare static " + PARAMETERS_METHOD_NAME + " method");
70-
}
71-
88+
Method method = findParametersMethod(testClass);
7289
if (!Modifier.isStatic(method.getModifiers())) {
7390
throw new ExtensionConfigurationException(PARAMETERS_METHOD_NAME + " method must be static");
7491
}

‎hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/FlushRegionProcedure.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public FlushRegionProcedure(RegionInfo region, List<byte[]> columnFamilies) {
7777
}
7878

7979
@Override
80-
protected Procedure<MasterProcedureEnv>[] execute(MasterProcedureEnv env)
80+
protected synchronized Procedure<MasterProcedureEnv>[] execute(MasterProcedureEnv env)
8181
throws ProcedureYieldException, ProcedureSuspendedException, InterruptedException {
8282
if (dispatched) {
8383
if (succ) {
@@ -158,7 +158,7 @@ public void remoteOperationFailed(MasterProcedureEnv env, RemoteProcedureExcepti
158158
complete(env, error);
159159
}
160160

161-
private void complete(MasterProcedureEnv env, Throwable error) {
161+
private synchronized void complete(MasterProcedureEnv env, Throwable error) {
162162
if (isFinished()) {
163163
LOG.info("This procedure {} is already finished, skip the rest processes", this.getProcId());
164164
return;

‎hbase-server/src/test/java/org/apache/hadoop/hbase/AcidGuaranteesTestBase.java‎

Lines changed: 30 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static org.apache.hadoop.hbase.AcidGuaranteesTestTool.FAMILIES;
2121
import static org.apache.hadoop.hbase.AcidGuaranteesTestTool.TABLE_NAME;
2222

23+
import java.util.Arrays;
2324
import java.util.List;
2425
import java.util.stream.Stream;
2526
import org.apache.hadoop.conf.Configuration;
@@ -28,11 +29,11 @@
2829
import org.apache.hadoop.hbase.regionserver.CompactingMemStore;
2930
import org.apache.hadoop.hbase.regionserver.ConstantSizeRegionSplitPolicy;
3031
import org.apache.hadoop.hbase.regionserver.MemStoreLAB;
31-
import org.junit.After;
32-
import org.junit.AfterClass;
33-
import org.junit.Before;
34-
import org.junit.BeforeClass;
35-
import org.junit.Test;
32+
import org.junit.jupiter.api.AfterAll;
33+
import org.junit.jupiter.api.AfterEach;
34+
import org.junit.jupiter.api.BeforeAll;
35+
import org.junit.jupiter.api.BeforeEach;
36+
import org.junit.jupiter.params.provider.Arguments;
3637

3738
import org.apache.hbase.thirdparty.com.google.common.collect.Lists;
3839

@@ -47,9 +48,17 @@ public abstract class AcidGuaranteesTestBase {
4748

4849
private AcidGuaranteesTestTool tool = new AcidGuaranteesTestTool();
4950

50-
protected abstract MemoryCompactionPolicy getMemoryCompactionPolicy();
51+
private MemoryCompactionPolicy policy;
5152

52-
@BeforeClass
53+
protected AcidGuaranteesTestBase(MemoryCompactionPolicy policy) {
54+
this.policy = policy;
55+
}
56+
57+
public static Stream<Arguments> parameters() {
58+
return Arrays.stream(MemoryCompactionPolicy.values()).map(Arguments::of);
59+
}
60+
61+
@BeforeAll
5362
public static void setUpBeforeClass() throws Exception {
5463
// Set small flush size for minicluster so we exercise reseeking scanners
5564
Configuration conf = UTIL.getConfiguration();
@@ -61,14 +70,13 @@ public static void setUpBeforeClass() throws Exception {
6170
UTIL.startMiniCluster(1);
6271
}
6372

64-
@AfterClass
73+
@AfterAll
6574
public static void tearDownAfterClass() throws Exception {
6675
UTIL.shutdownMiniCluster();
6776
}
6877

69-
@Before
78+
@BeforeEach
7079
public void setUp() throws Exception {
71-
MemoryCompactionPolicy policy = getMemoryCompactionPolicy();
7280
TableDescriptorBuilder builder = TableDescriptorBuilder.newBuilder(TABLE_NAME)
7381
.setValue(CompactingMemStore.COMPACTING_MEMSTORE_TYPE_KEY, policy.name());
7482
if (policy == MemoryCompactionPolicy.EAGER) {
@@ -77,22 +85,26 @@ public void setUp() throws Exception {
7785
}
7886
Stream.of(FAMILIES).map(ColumnFamilyDescriptorBuilder::of)
7987
.forEachOrdered(builder::setColumnFamily);
88+
for (int i = 0; i < 10; i++) {
89+
// try to delete the table several times
90+
if (UTIL.getAdmin().tableExists(TABLE_NAME)) {
91+
UTIL.deleteTable(TABLE_NAME);
92+
Thread.sleep(1000);
93+
} else {
94+
break;
95+
}
96+
}
8097
UTIL.getAdmin().createTable(builder.build());
8198
tool.setConf(UTIL.getConfiguration());
8299
}
83100

84-
@After
101+
@AfterEach
85102
public void tearDown() throws Exception {
86103
UTIL.deleteTable(TABLE_NAME);
87104
}
88105

89-
private void runTestAtomicity(long millisToRun, int numWriters, int numGetters, int numScanners,
90-
int numUniqueRows) throws Exception {
91-
runTestAtomicity(millisToRun, numWriters, numGetters, numScanners, numUniqueRows, false);
92-
}
93-
94-
private void runTestAtomicity(long millisToRun, int numWriters, int numGetters, int numScanners,
95-
int numUniqueRows, boolean useMob) throws Exception {
106+
protected final void runTestAtomicity(long millisToRun, int numWriters, int numGetters,
107+
int numScanners, int numUniqueRows, boolean useMob) throws Exception {
96108
List<String> args = Lists.newArrayList("-millis", String.valueOf(millisToRun), "-numWriters",
97109
String.valueOf(numWriters), "-numGetters", String.valueOf(numGetters), "-numScanners",
98110
String.valueOf(numScanners), "-numUniqueRows", String.valueOf(numUniqueRows), "-crazyFlush");
@@ -102,33 +114,4 @@ private void runTestAtomicity(long millisToRun, int numWriters, int numGetters,
102114
tool.run(args.toArray(new String[0]));
103115
}
104116

105-
@Test
106-
public void testGetAtomicity() throws Exception {
107-
runTestAtomicity(20000, 5, 5, 0, 3);
108-
}
109-
110-
@Test
111-
public void testScanAtomicity() throws Exception {
112-
runTestAtomicity(20000, 5, 0, 5, 3);
113-
}
114-
115-
@Test
116-
public void testMixedAtomicity() throws Exception {
117-
runTestAtomicity(20000, 5, 2, 2, 3);
118-
}
119-
120-
@Test
121-
public void testMobGetAtomicity() throws Exception {
122-
runTestAtomicity(20000, 5, 5, 0, 3, true);
123-
}
124-
125-
@Test
126-
public void testMobScanAtomicity() throws Exception {
127-
runTestAtomicity(20000, 5, 0, 5, 3, true);
128-
}
129-
130-
@Test
131-
public void testMobMixedAtomicity() throws Exception {
132-
runTestAtomicity(20000, 5, 2, 2, 3, true);
133-
}
134117
}

hbase-server/src/test/java/org/apache/hadoop/hbase/TestAcidGuaranteesWithEagerPolicy.java renamed to hbase-server/src/test/java/org/apache/hadoop/hbase/TestGetAtomicity.java

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,19 @@
1818
package org.apache.hadoop.hbase;
1919

2020
import org.apache.hadoop.hbase.testclassification.LargeTests;
21-
import org.junit.ClassRule;
22-
import org.junit.experimental.categories.Category;
21+
import org.junit.jupiter.api.Tag;
22+
import org.junit.jupiter.api.TestTemplate;
2323

24-
@Category(LargeTests.class)
25-
public class TestAcidGuaranteesWithEagerPolicy extends AcidGuaranteesTestBase {
24+
@Tag(LargeTests.TAG)
25+
@HBaseParameterizedTestTemplate(name = "{index}: policy = {0}")
26+
public class TestGetAtomicity extends AcidGuaranteesTestBase {
2627

27-
@ClassRule
28-
public static final HBaseClassTestRule CLASS_RULE =
29-
HBaseClassTestRule.forClass(TestAcidGuaranteesWithEagerPolicy.class);
28+
public TestGetAtomicity(MemoryCompactionPolicy policy) {
29+
super(policy);
30+
}
3031

31-
@Override
32-
protected MemoryCompactionPolicy getMemoryCompactionPolicy() {
33-
return MemoryCompactionPolicy.EAGER;
32+
@TestTemplate
33+
public void testGetAtomicity() throws Exception {
34+
runTestAtomicity(20000, 5, 5, 0, 3, false);
3435
}
3536
}

hbase-server/src/test/java/org/apache/hadoop/hbase/TestAcidGuaranteesWithBasicPolicy.java renamed to hbase-server/src/test/java/org/apache/hadoop/hbase/TestMixedAtomicity.java

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,19 @@
1818
package org.apache.hadoop.hbase;
1919

2020
import org.apache.hadoop.hbase.testclassification.LargeTests;
21-
import org.junit.ClassRule;
22-
import org.junit.experimental.categories.Category;
21+
import org.junit.jupiter.api.Tag;
22+
import org.junit.jupiter.api.TestTemplate;
2323

24-
@Category(LargeTests.class)
25-
public class TestAcidGuaranteesWithBasicPolicy extends AcidGuaranteesTestBase {
24+
@Tag(LargeTests.TAG)
25+
@HBaseParameterizedTestTemplate(name = "{index}: policy = {0}")
26+
public class TestMixedAtomicity extends AcidGuaranteesTestBase {
2627

27-
@ClassRule
28-
public static final HBaseClassTestRule CLASS_RULE =
29-
HBaseClassTestRule.forClass(TestAcidGuaranteesWithBasicPolicy.class);
28+
public TestMixedAtomicity(MemoryCompactionPolicy policy) {
29+
super(policy);
30+
}
3031

31-
@Override
32-
protected MemoryCompactionPolicy getMemoryCompactionPolicy() {
33-
return MemoryCompactionPolicy.BASIC;
32+
@TestTemplate
33+
public void testMixedAtomicity() throws Exception {
34+
runTestAtomicity(20000, 5, 2, 2, 3, false);
3435
}
3536
}

hbase-server/src/test/java/org/apache/hadoop/hbase/TestAcidGuaranteesWithAdaptivePolicy.java renamed to hbase-server/src/test/java/org/apache/hadoop/hbase/TestMobGetAtomicity.java

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,19 @@
1818
package org.apache.hadoop.hbase;
1919

2020
import org.apache.hadoop.hbase.testclassification.LargeTests;
21-
import org.junit.ClassRule;
22-
import org.junit.experimental.categories.Category;
21+
import org.junit.jupiter.api.Tag;
22+
import org.junit.jupiter.api.TestTemplate;
2323

24-
@Category(LargeTests.class)
25-
public class TestAcidGuaranteesWithAdaptivePolicy extends AcidGuaranteesTestBase {
24+
@Tag(LargeTests.TAG)
25+
@HBaseParameterizedTestTemplate(name = "{index}: policy = {0}")
26+
public class TestMobGetAtomicity extends AcidGuaranteesTestBase {
2627

27-
@ClassRule
28-
public static final HBaseClassTestRule CLASS_RULE =
29-
HBaseClassTestRule.forClass(TestAcidGuaranteesWithAdaptivePolicy.class);
28+
public TestMobGetAtomicity(MemoryCompactionPolicy policy) {
29+
super(policy);
30+
}
3031

31-
@Override
32-
protected MemoryCompactionPolicy getMemoryCompactionPolicy() {
33-
return MemoryCompactionPolicy.ADAPTIVE;
32+
@TestTemplate
33+
public void testMobScanAtomicity() throws Exception {
34+
runTestAtomicity(20000, 5, 0, 5, 3, true);
3435
}
3536
}

hbase-server/src/test/java/org/apache/hadoop/hbase/TestAcidGuaranteesWithNoInMemCompaction.java renamed to hbase-server/src/test/java/org/apache/hadoop/hbase/TestMobMixedAtomicity.java

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,19 @@
1818
package org.apache.hadoop.hbase;
1919

2020
import org.apache.hadoop.hbase.testclassification.LargeTests;
21-
import org.junit.ClassRule;
22-
import org.junit.experimental.categories.Category;
21+
import org.junit.jupiter.api.Tag;
22+
import org.junit.jupiter.api.TestTemplate;
2323

24-
@Category(LargeTests.class)
25-
public class TestAcidGuaranteesWithNoInMemCompaction extends AcidGuaranteesTestBase {
24+
@Tag(LargeTests.TAG)
25+
@HBaseParameterizedTestTemplate(name = "{index}: policy = {0}")
26+
public class TestMobMixedAtomicity extends AcidGuaranteesTestBase {
2627

27-
@ClassRule
28-
public static final HBaseClassTestRule CLASS_RULE =
29-
HBaseClassTestRule.forClass(TestAcidGuaranteesWithNoInMemCompaction.class);
28+
public TestMobMixedAtomicity(MemoryCompactionPolicy policy) {
29+
super(policy);
30+
}
3031

31-
@Override
32-
protected MemoryCompactionPolicy getMemoryCompactionPolicy() {
33-
return MemoryCompactionPolicy.NONE;
32+
@TestTemplate
33+
public void testMobMixedAtomicity() throws Exception {
34+
runTestAtomicity(20000, 5, 2, 2, 3, true);
3435
}
3536
}

‎hbase-server/src/test/java/org/apache/hadoop/hbase/TestMobScanAtomicity.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.apache.hadoop.hbase;
19+
20+
import org.apache.hadoop.hbase.testclassification.LargeTests;
21+
import org.junit.jupiter.api.Tag;
22+
import org.junit.jupiter.api.TestTemplate;
23+
24+
@Tag(LargeTests.TAG)
25+
@HBaseParameterizedTestTemplate(name = "{index}: policy = {0}")
26+
public class TestMobScanAtomicity extends AcidGuaranteesTestBase {
27+
28+
public TestMobScanAtomicity(MemoryCompactionPolicy policy) {
29+
super(policy);
30+
}
31+
32+
@TestTemplate
33+
public void testMobGetAtomicity() throws Exception {
34+
runTestAtomicity(20000, 5, 5, 0, 3, true);
35+
}
36+
}

0 commit comments

Comments
 (0)