Skip to content

Commit 1873dcf

Browse files
cuibo01cuibo01
committed
HBASE-24960 reduce invalid subprocedure task
1 parent 3537f91 commit 1873dcf

File tree

4 files changed

+249
-0
lines changed

4 files changed

+249
-0
lines changed

‎hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/flush/RegionServerFlushTableProcedureManager.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.apache.hadoop.hbase.util.Threads;
4848
import org.apache.hadoop.hbase.zookeeper.ZKWatcher;
4949
import org.apache.hbase.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder;
50+
import org.apache.hbase.thirdparty.org.apache.commons.collections4.CollectionUtils;
5051
import org.apache.zookeeper.KeeperException;
5152

5253
import org.apache.yetus.audience.InterfaceAudience;
@@ -150,6 +151,11 @@ public Subprocedure buildSubprocedure(String table, String family) {
150151
throw new IllegalStateException("Failed to figure out if there is region to flush.", e1);
151152
}
152153

154+
if (CollectionUtils.isEmpty(involvedRegions)) {
155+
LOG.info("no region of {} is online on the {}.", table, this.rss.getServerName());
156+
return null;
157+
}
158+
153159
// We need to run the subprocedure even if we have no relevant regions. The coordinator
154160
// expects participation in the procedure and without sending message the master procedure
155161
// will hang and fail.

‎hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/snapshot/RegionServerSnapshotManager.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.apache.hadoop.hbase.util.Threads;
3838
import org.apache.hadoop.hbase.zookeeper.ZKWatcher;
3939
import org.apache.hbase.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder;
40+
import org.apache.hbase.thirdparty.org.apache.commons.collections4.CollectionUtils;
4041
import org.apache.yetus.audience.InterfaceAudience;
4142
import org.apache.yetus.audience.InterfaceStability;
4243
import org.apache.hadoop.hbase.client.RegionReplicaUtil;
@@ -170,6 +171,11 @@ public Subprocedure buildSubprocedure(SnapshotDescription snapshot) {
170171
+ "something has gone awry with the online regions.", e1);
171172
}
172173

174+
if (CollectionUtils.isEmpty(involvedRegions)) {
175+
LOG.info("no region of {} is online on the {}.", snapshot.getTable(), this.rss.getServerName());
176+
return null;
177+
}
178+
173179
// We need to run the subprocedure even if we have no relevant regions. The coordinator
174180
// expects participation in the procedure and without sending message the snapshot attempt
175181
// will hang and fail.

‎hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/flush/TestRegionServerFlushTableProcedureManager.java

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
package org.apache.hadoop.hbase.procedure.flush;
2+
3+
import org.apache.hadoop.hbase.HBaseClassTestRule;
4+
import org.apache.hadoop.hbase.HBaseTestingUtility;
5+
import org.apache.hadoop.hbase.HRegionLocation;
6+
import org.apache.hadoop.hbase.ServerName;
7+
import org.apache.hadoop.hbase.TableName;
8+
import org.apache.hadoop.hbase.client.Admin;
9+
import org.apache.hadoop.hbase.client.Connection;
10+
import org.apache.hadoop.hbase.procedure.ProcedureMember;
11+
import org.apache.hadoop.hbase.procedure.RegionServerProcedureManagerHost;
12+
import org.apache.hadoop.hbase.procedure.Subprocedure;
13+
import org.apache.hadoop.hbase.procedure.SubprocedureFactory;
14+
import org.apache.hadoop.hbase.regionserver.HRegion;
15+
import org.apache.hadoop.hbase.regionserver.HRegionServer;
16+
import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos;
17+
import org.apache.hadoop.hbase.util.Bytes;
18+
import org.apache.hbase.thirdparty.com.google.protobuf.InvalidProtocolBufferException;
19+
import org.junit.AfterClass;
20+
import org.junit.BeforeClass;
21+
import org.junit.ClassRule;
22+
import org.junit.Test;
23+
import org.slf4j.Logger;
24+
import org.slf4j.LoggerFactory;
25+
import java.io.IOException;
26+
import java.lang.reflect.Field;
27+
import java.util.List;
28+
import static org.junit.Assert.*;
29+
30+
public class TestRegionServerFlushTableProcedureManager {
31+
@ClassRule
32+
public static final HBaseClassTestRule CLASS_RULE =
33+
HBaseClassTestRule.forClass(TestRegionServerFlushTableProcedureManager.class);
34+
private static final Logger LOG = LoggerFactory.getLogger(TestRegionServerFlushTableProcedureManager.class);
35+
private static HBaseTestingUtility TEST_UTIL;
36+
private static Connection connection;
37+
private static Admin admin;
38+
private static Boolean hasError = false;
39+
@BeforeClass
40+
public static void setupBeforeClass() throws Exception {
41+
TEST_UTIL = new HBaseTestingUtility();
42+
TEST_UTIL.startMiniCluster(2);
43+
TEST_UTIL.getHBaseCluster().waitForActiveAndReadyMaster();
44+
TEST_UTIL.waitUntilAllRegionsAssigned(TableName.NAMESPACE_TABLE_NAME);
45+
connection = TEST_UTIL.getConnection();
46+
admin = connection.getAdmin();
47+
}
48+
49+
@AfterClass
50+
public static void tearDown() throws Exception {
51+
TEST_UTIL.shutdownMiniCluster();
52+
}
53+
54+
private int createAndLoadTable(TableName tableName) throws IOException {
55+
TEST_UTIL.createTable(tableName,new byte[][]{ Bytes.toBytes("fam")});
56+
TEST_UTIL.loadTable(connection.getTable(tableName), Bytes.toBytes("fam"));
57+
return TEST_UTIL.countRows(tableName);
58+
}
59+
60+
private Object setAndGetField(Object object, String field, Object value)
61+
throws IllegalAccessException, NoSuchFieldException {
62+
Field f = null;
63+
try {
64+
f = object.getClass().getField(field);
65+
} catch (NoSuchFieldException e) {
66+
f = object.getClass().getSuperclass().getField(field);
67+
}
68+
f.setAccessible(true);
69+
if (value != null) {
70+
f.set(object, value);
71+
}
72+
return f.get(object);
73+
}
74+
75+
private void setRSSnapshotProcManagerMock(HRegionServer regionServer, boolean hasRegion)
76+
throws NoSuchFieldException, IllegalAccessException {
77+
RegionServerProcedureManagerHost
78+
rspmHost = (RegionServerProcedureManagerHost) setAndGetField(regionServer, "rspmHost", null);
79+
RegionServerFlushTableProcedureManager rsManager = rspmHost.getProcedureManagers().stream().filter(v -> v instanceof RegionServerFlushTableProcedureManager)
80+
.map(v -> (RegionServerFlushTableProcedureManager)v).findAny().get();
81+
ProcedureMember procedureMember = (ProcedureMember) setAndGetField(rsManager, "member", null);
82+
setAndGetField(procedureMember, "builder", new SubprocedureFactory() {
83+
@Override
84+
public Subprocedure buildSubprocedure(String procName, byte[] procArgs) {
85+
String family = null;
86+
// Currently we do not put other data except family, so it is ok to
87+
// judge by length that if family was specified
88+
if (procArgs.length > 0) {
89+
try {
90+
HBaseProtos.NameStringPair nsp = HBaseProtos.NameStringPair.parseFrom(procArgs);
91+
family = nsp.getValue();
92+
} catch (Exception e) {
93+
LOG.error("fail to get family by parsing from data", e);
94+
hasError |= true;
95+
}
96+
}
97+
Subprocedure subprocedure = rsManager.buildSubprocedure(procName, family);
98+
hasError |= (hasRegion && subprocedure == null || !hasRegion && subprocedure != null);
99+
return subprocedure;
100+
}
101+
});
102+
}
103+
104+
@Test
105+
public void testInvalidSubProcedure()
106+
throws IOException, NoSuchFieldException, IllegalAccessException, InterruptedException {
107+
TableName tableName = TableName.valueOf("test_table");
108+
int count = createAndLoadTable(tableName);
109+
List<HRegionLocation> regionLocationList = connection.getRegionLocator(tableName).getAllRegionLocations();
110+
ServerName serverName = regionLocationList.stream().map(v -> v.getServerName()).findAny().get();
111+
String regionName = regionLocationList.get(0).getRegion().getEncodedName();
112+
HRegion region = TEST_UTIL.getRSForFirstRegionInTable(tableName).getRegion(regionName);
113+
HRegionServer regionServer0 = TEST_UTIL.getHBaseCluster().getRegionServer(0);
114+
HRegionServer regionServer1 = TEST_UTIL.getHBaseCluster().getRegionServer(1);
115+
setRSSnapshotProcManagerMock(regionServer0, regionServer0.getServerName().equals(serverName));
116+
setRSSnapshotProcManagerMock(regionServer1, regionServer1.getServerName().equals(serverName));
117+
assertNotEquals(0, region.getMemStoreDataSize());
118+
admin.flush(tableName);
119+
assertFalse(hasError);
120+
assertEquals(0, region.getMemStoreDataSize());
121+
assertEquals(count, TEST_UTIL.countRows(tableName));
122+
}
123+
}

‎hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/snapshot/TestRegionServerSnapshotManager.java

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
package org.apache.hadoop.hbase.regionserver.snapshot;
2+
3+
import org.apache.hadoop.hbase.HBaseClassTestRule;
4+
import org.apache.hadoop.hbase.HBaseTestingUtility;
5+
import org.apache.hadoop.hbase.ServerName;
6+
import org.apache.hadoop.hbase.TableName;
7+
import org.apache.hadoop.hbase.client.Admin;
8+
import org.apache.hadoop.hbase.client.Connection;
9+
import org.apache.hadoop.hbase.procedure.ProcedureMember;
10+
import org.apache.hadoop.hbase.procedure.RegionServerProcedureManagerHost;
11+
import org.apache.hadoop.hbase.procedure.Subprocedure;
12+
import org.apache.hadoop.hbase.procedure.SubprocedureFactory;
13+
import org.apache.hadoop.hbase.regionserver.HRegionServer;
14+
import org.apache.hadoop.hbase.shaded.protobuf.generated.SnapshotProtos.SnapshotDescription;
15+
import org.apache.hadoop.hbase.testclassification.MediumTests;
16+
import org.apache.hadoop.hbase.testclassification.RegionServerTests;
17+
import org.apache.hadoop.hbase.util.Bytes;
18+
import org.apache.hbase.thirdparty.com.google.protobuf.InvalidProtocolBufferException;
19+
import org.junit.AfterClass;
20+
import org.junit.BeforeClass;
21+
import org.junit.ClassRule;
22+
import org.junit.Test;
23+
import org.junit.experimental.categories.Category;
24+
import java.io.IOException;
25+
import java.lang.reflect.Field;
26+
import static org.junit.Assert.*;
27+
28+
@Category({MediumTests.class, RegionServerTests.class })
29+
public class TestRegionServerSnapshotManager {
30+
@ClassRule
31+
public static final HBaseClassTestRule CLASS_RULE =
32+
HBaseClassTestRule.forClass(TestRegionServerSnapshotManager.class);
33+
private static HBaseTestingUtility TEST_UTIL;
34+
private static Connection connection;
35+
private static Admin admin;
36+
private static Boolean hasError = false;
37+
@BeforeClass
38+
public static void setupBeforeClass() throws Exception {
39+
TEST_UTIL = new HBaseTestingUtility();
40+
TEST_UTIL.startMiniCluster(2);
41+
TEST_UTIL.getHBaseCluster().waitForActiveAndReadyMaster();
42+
TEST_UTIL.waitUntilAllRegionsAssigned(TableName.NAMESPACE_TABLE_NAME);
43+
connection = TEST_UTIL.getConnection();
44+
admin = connection.getAdmin();
45+
}
46+
47+
@AfterClass
48+
public static void tearDown() throws Exception {
49+
TEST_UTIL.shutdownMiniCluster();
50+
}
51+
52+
private int createAndLoadTable(TableName tableName) throws IOException {
53+
TEST_UTIL.createTable(tableName,new byte[][]{Bytes.toBytes("fam")});
54+
TEST_UTIL.loadTable(connection.getTable(tableName), Bytes.toBytes("fam"));
55+
return TEST_UTIL.countRows(tableName);
56+
}
57+
58+
private Object setAndGetField(Object object, String field, Object value)
59+
throws IllegalAccessException, NoSuchFieldException {
60+
Field f = null;
61+
try {
62+
f = object.getClass().getField(field);
63+
} catch (NoSuchFieldException e) {
64+
f = object.getClass().getSuperclass().getField(field);
65+
}
66+
f.setAccessible(true);
67+
if (value != null) {
68+
f.set(object, value);
69+
}
70+
return f.get(object);
71+
}
72+
73+
private void setRSSnapshotProcManagerMock(HRegionServer regionServer, boolean hasRegion)
74+
throws NoSuchFieldException, IllegalAccessException {
75+
RegionServerProcedureManagerHost rspmHost = (RegionServerProcedureManagerHost) setAndGetField(regionServer, "rspmHost", null);
76+
RegionServerSnapshotManager rsManager = rspmHost.getProcedureManagers().stream().filter(v -> v instanceof RegionServerSnapshotManager)
77+
.map(v -> (RegionServerSnapshotManager)v).findAny().get();
78+
ProcedureMember procedureMember = (ProcedureMember) setAndGetField(rsManager, "member", null);
79+
setAndGetField(procedureMember, "builder", new SubprocedureFactory() {
80+
@Override
81+
public Subprocedure buildSubprocedure(String procName, byte[] procArgs) {
82+
SnapshotDescription snapshot = null;
83+
try {
84+
snapshot = SnapshotDescription.parseFrom(procArgs);
85+
} catch (InvalidProtocolBufferException e) {
86+
throw new IllegalArgumentException("Could not read snapshot information from request.");
87+
}
88+
Subprocedure subprocedure = rsManager.buildSubprocedure(snapshot);
89+
hasError |= (hasRegion && subprocedure == null || !hasRegion && subprocedure != null);
90+
return subprocedure;
91+
}
92+
});
93+
}
94+
95+
@Test
96+
public void testInvalidSubProcedure()
97+
throws IOException, NoSuchFieldException, IllegalAccessException {
98+
TableName tableName = TableName.valueOf("test_table");
99+
int count = createAndLoadTable(tableName);
100+
ServerName serverName = connection.getRegionLocator(tableName).getAllRegionLocations().stream()
101+
.map(v -> v.getServerName()).findAny().get();
102+
HRegionServer regionServer0 = TEST_UTIL.getHBaseCluster().getRegionServer(0);
103+
HRegionServer regionServer1 = TEST_UTIL.getHBaseCluster().getRegionServer(1);
104+
setRSSnapshotProcManagerMock(regionServer0, regionServer0.getServerName().equals(serverName));
105+
setRSSnapshotProcManagerMock(regionServer1, regionServer1.getServerName().equals(serverName));
106+
admin.snapshot("ss", tableName);
107+
assertFalse(hasError);
108+
admin.disableTable(tableName);
109+
admin.deleteTable(tableName);
110+
admin.cloneSnapshot("ss", tableName);
111+
assertEquals(count, TEST_UTIL.countRows(tableName));
112+
}
113+
114+
}

0 commit comments

Comments
 (0)