Skip to content

HBASE-22551 TestMasterOperationsForRegionReplicas is flakey #295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Apache9 merged 1 commit into from
Jun 9, 2019
Merged

HBASE-22551 TestMasterOperationsForRegionReplicas is flakey #295

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
184 changes: 98 additions & 86 deletions ...r/src/test/java/org/apache/hadoop/hbase/master/TestMasterOperationsForRegionReplicas.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
package org.apache.hadoop.hbase.master;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

import java.io.IOException;
Expand All @@ -35,24 +37,25 @@
import org.apache.hadoop.hbase.ClusterMetrics.Option;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HBaseTestingUtility;
import org.apache.hadoop.hbase.HColumnDescriptor;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.HRegionLocation;
import org.apache.hadoop.hbase.HTableDescriptor;
import org.apache.hadoop.hbase.MetaTableAccessor;
import org.apache.hadoop.hbase.MetaTableAccessor.Visitor;
import org.apache.hadoop.hbase.RegionLocations;
import org.apache.hadoop.hbase.ServerName;
import org.apache.hadoop.hbase.StartMiniClusterOption;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.Admin;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
import org.apache.hadoop.hbase.client.Connection;
import org.apache.hadoop.hbase.client.ConnectionFactory;
import org.apache.hadoop.hbase.client.Delete;
import org.apache.hadoop.hbase.client.RegionInfo;
import org.apache.hadoop.hbase.client.RegionReplicaUtil;
import org.apache.hadoop.hbase.client.Result;
import org.apache.hadoop.hbase.client.Table;
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
import org.apache.hadoop.hbase.testclassification.MasterTests;
import org.apache.hadoop.hbase.testclassification.MediumTests;
import org.apache.hadoop.hbase.util.Bytes;
Expand All @@ -68,12 +71,14 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@Category({MasterTests.class, MediumTests.class})
import org.apache.hbase.thirdparty.com.google.common.io.Closeables;

@Category({ MasterTests.class, MediumTests.class })
public class TestMasterOperationsForRegionReplicas {

@ClassRule
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestMasterOperationsForRegionReplicas.class);
HBaseClassTestRule.forClass(TestMasterOperationsForRegionReplicas.class);

private static final Logger LOG = LoggerFactory.getLogger(TestRegionPlacement.class);
private final static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
Expand All @@ -90,18 +95,19 @@ public static void setupBeforeClass() throws Exception {
conf = TEST_UTIL.getConfiguration();
conf.setBoolean("hbase.tests.use.shortcircuit.reads", false);
TEST_UTIL.startMiniCluster(numSlaves);
TEST_UTIL.getAdmin().balancerSwitch(false, true);
CONNECTION = ConnectionFactory.createConnection(TEST_UTIL.getConfiguration());
ADMIN = CONNECTION.getAdmin();
while(ADMIN.getClusterMetrics(EnumSet.of(Option.LIVE_SERVERS))
.getLiveServerMetrics().size() < numSlaves) {
while (ADMIN.getClusterMetrics(EnumSet.of(Option.LIVE_SERVERS)).getLiveServerMetrics()
.size() < numSlaves) {
Thread.sleep(100);
}
}

@AfterClass
public static void tearDownAfterClass() throws Exception {
if (ADMIN != null) ADMIN.close();
if (CONNECTION != null && !CONNECTION.isClosed()) CONNECTION.close();
Closeables.close(ADMIN, true);
Closeables.close(CONNECTION, true);

@saintstack saintstack Jun 9, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice utility...

TEST_UTIL.shutdownMiniCluster();
}

Expand All @@ -111,15 +117,16 @@ public void testCreateTableWithSingleReplica() throws Exception {
final int numReplica = 1;
final TableName tableName = TableName.valueOf(name.getMethodName());
try {
HTableDescriptor desc = new HTableDescriptor(tableName);
desc.setRegionReplication(numReplica);
desc.addFamily(new HColumnDescriptor("family"));
TableDescriptor desc =
TableDescriptorBuilder.newBuilder(tableName).setRegionReplication(numReplica)
.setColumnFamily(ColumnFamilyDescriptorBuilder.of("family")).build();
ADMIN.createTable(desc, Bytes.toBytes("A"), Bytes.toBytes("Z"), numRegions);
TEST_UTIL.waitUntilAllRegionsAssigned(tableName);
TEST_UTIL.waitUntilNoRegionsInTransition();

validateNumberOfRowsInMeta(tableName, numRegions, ADMIN.getConnection());
List<RegionInfo> hris = MetaTableAccessor.getTableRegions(
ADMIN.getConnection(), tableName);
assert(hris.size() == numRegions * numReplica);
List<RegionInfo> hris = MetaTableAccessor.getTableRegions(ADMIN.getConnection(), tableName);
assertEquals(numRegions * numReplica, hris.size());
} finally {
ADMIN.disableTable(tableName);
ADMIN.deleteTable(tableName);
Expand All @@ -132,38 +139,39 @@ public void testCreateTableWithMultipleReplicas() throws Exception {
final int numRegions = 3;
final int numReplica = 2;
try {
HTableDescriptor desc = new HTableDescriptor(tableName);
desc.setRegionReplication(numReplica);
desc.addFamily(new HColumnDescriptor("family"));
TableDescriptor desc =
TableDescriptorBuilder.newBuilder(tableName).setRegionReplication(numReplica)
.setColumnFamily(ColumnFamilyDescriptorBuilder.of("family")).build();
ADMIN.createTable(desc, Bytes.toBytes("A"), Bytes.toBytes("Z"), numRegions);
TEST_UTIL.waitTableEnabled(tableName);
TEST_UTIL.waitUntilAllRegionsAssigned(tableName);
TEST_UTIL.waitUntilNoRegionsInTransition();
validateNumberOfRowsInMeta(tableName, numRegions, ADMIN.getConnection());

List<RegionInfo> hris = MetaTableAccessor.getTableRegions(ADMIN.getConnection(), tableName);
assert(hris.size() == numRegions * numReplica);
assertEquals(numRegions * numReplica, hris.size());
// check that the master created expected number of RegionState objects
for (int i = 0; i < numRegions; i++) {
for (int j = 0; j < numReplica; j++) {
RegionInfo replica = RegionReplicaUtil.getRegionInfoForReplica(hris.get(i), j);
RegionState state = TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager()
.getRegionStates().getRegionState(replica);
assert (state != null);
.getRegionStates().getRegionState(replica);
assertNotNull(state);
}
}

List<Result> metaRows = MetaTableAccessor.fullScanRegions(ADMIN.getConnection());
int numRows = 0;
for (Result result : metaRows) {
RegionLocations locations = MetaTableAccessor.getRegionLocations(result);
RegionInfo hri = locations.getRegionLocation().getRegionInfo();
RegionInfo hri = locations.getRegionLocation().getRegion();
if (!hri.getTable().equals(tableName)) continue;
numRows += 1;
HRegionLocation[] servers = locations.getRegionLocations();
// have two locations for the replicas of a region, and the locations should be different
assert(servers.length == 2);
assert(!servers[0].equals(servers[1]));
assertEquals(2, servers.length);
assertNotEquals(servers[1], servers[0]);
}
assert(numRows == numRegions);
assertEquals(numRegions, numRows);

// The same verification of the meta as above but with the SnapshotOfRegionAssignmentFromMeta
// class
Expand All @@ -176,12 +184,14 @@ public void testCreateTableWithMultipleReplicas() throws Exception {
TEST_UTIL.getHBaseClusterInterface().waitForMasterToStop(master, 30000);
TEST_UTIL.getHBaseClusterInterface().startMaster(master.getHostname(), master.getPort());
TEST_UTIL.getHBaseClusterInterface().waitForActiveAndReadyMaster();
TEST_UTIL.waitUntilAllRegionsAssigned(tableName);
TEST_UTIL.waitUntilNoRegionsInTransition();
for (int i = 0; i < numRegions; i++) {
for (int j = 0; j < numReplica; j++) {
RegionInfo replica = RegionReplicaUtil.getRegionInfoForReplica(hris.get(i), j);
RegionState state = TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager()
.getRegionStates().getRegionState(replica);
assert (state != null);
.getRegionStates().getRegionState(replica);
assertNotNull(state);
}
}
validateFromSnapshotFromMeta(TEST_UTIL, tableName, numRegions, numReplica,
Expand All @@ -192,86 +202,94 @@ public void testCreateTableWithMultipleReplicas() throws Exception {
// figure current cluster ports and pass them in on next cluster start so new cluster comes
// up at same coordinates -- and the assignment retention logic has a chance to cut in.
List<Integer> rsports = new ArrayList<>();
for (JVMClusterUtil.RegionServerThread rst:
TEST_UTIL.getHBaseCluster().getLiveRegionServerThreads()) {
for (JVMClusterUtil.RegionServerThread rst : TEST_UTIL.getHBaseCluster()
.getLiveRegionServerThreads()) {
rsports.add(rst.getRegionServer().getRpcServer().getListenerAddress().getPort());
}
TEST_UTIL.shutdownMiniHBaseCluster();
StartMiniClusterOption option = StartMiniClusterOption.builder()
.numRegionServers(numSlaves).rsPorts(rsports).build();
StartMiniClusterOption option =
StartMiniClusterOption.builder().numRegionServers(numSlaves).rsPorts(rsports).build();
TEST_UTIL.startMiniHBaseCluster(option);
TEST_UTIL.waitTableAvailable(tableName);
TEST_UTIL.waitUntilAllRegionsAssigned(tableName);
TEST_UTIL.waitUntilNoRegionsInTransition();
validateFromSnapshotFromMeta(TEST_UTIL, tableName, numRegions, numReplica,
ADMIN.getConnection());

// Now shut the whole cluster down, and verify regions are assigned even if there is only
// one server running
TEST_UTIL.shutdownMiniHBaseCluster();
TEST_UTIL.startMiniHBaseCluster();
TEST_UTIL.waitTableAvailable(tableName);
TEST_UTIL.waitUntilAllRegionsAssigned(tableName);
TEST_UTIL.waitUntilNoRegionsInTransition();
validateSingleRegionServerAssignment(ADMIN.getConnection(), numRegions, numReplica);
for (int i = 1; i < numSlaves; i++) { //restore the cluster
for (int i = 1; i < numSlaves; i++) { // restore the cluster
TEST_UTIL.getMiniHBaseCluster().startRegionServer();
}

// Check on alter table
ADMIN.disableTable(tableName);
assert(ADMIN.isTableDisabled(tableName));
//increase the replica
desc.setRegionReplication(numReplica + 1);
ADMIN.modifyTable(desc);
assertTrue(ADMIN.isTableDisabled(tableName));
// increase the replica
ADMIN.modifyTable(
TableDescriptorBuilder.newBuilder(desc).setRegionReplication(numReplica + 1).build());
ADMIN.enableTable(tableName);
LOG.info(ADMIN.getDescriptor(tableName).toString());
assert(ADMIN.isTableEnabled(tableName));
List<RegionInfo> regions = TEST_UTIL.getMiniHBaseCluster().getMaster().
getAssignmentManager().getRegionStates().getRegionsOfTable(tableName);
assertTrue("regions.size=" + regions.size() + ", numRegions=" + numRegions +
", numReplica=" + numReplica, regions.size() == numRegions * (numReplica + 1));
assertTrue(ADMIN.isTableEnabled(tableName));
TEST_UTIL.waitUntilAllRegionsAssigned(tableName);
TEST_UTIL.waitUntilNoRegionsInTransition();
List<RegionInfo> regions = TEST_UTIL.getMiniHBaseCluster().getMaster().getAssignmentManager()
.getRegionStates().getRegionsOfTable(tableName);
assertTrue("regions.size=" + regions.size() + ", numRegions=" + numRegions + ", numReplica=" +
numReplica, regions.size() == numRegions * (numReplica + 1));

//decrease the replica(earlier, table was modified to have a replica count of numReplica + 1)
// decrease the replica(earlier, table was modified to have a replica count of numReplica + 1)
ADMIN.disableTable(tableName);
desc.setRegionReplication(numReplica);
ADMIN.modifyTable(desc);
ADMIN.modifyTable(
TableDescriptorBuilder.newBuilder(desc).setRegionReplication(numReplica).build());
ADMIN.enableTable(tableName);
assert(ADMIN.isTableEnabled(tableName));
regions = TEST_UTIL.getMiniHBaseCluster().getMaster()
.getAssignmentManager().getRegionStates().getRegionsOfTable(tableName);
assert(regions.size() == numRegions * numReplica);
//also make sure the meta table has the replica locations removed
assertTrue(ADMIN.isTableEnabled(tableName));
TEST_UTIL.waitUntilAllRegionsAssigned(tableName);
TEST_UTIL.waitUntilNoRegionsInTransition();
regions = TEST_UTIL.getMiniHBaseCluster().getMaster().getAssignmentManager().getRegionStates()
.getRegionsOfTable(tableName);
assertEquals(numRegions * numReplica, regions.size());
// also make sure the meta table has the replica locations removed
hris = MetaTableAccessor.getTableRegions(ADMIN.getConnection(), tableName);
assert(hris.size() == numRegions * numReplica);
//just check that the number of default replica regions in the meta table are the same
//as the number of regions the table was created with, and the count of the
//replicas is numReplica for each region
assertEquals(numRegions * numReplica, hris.size());
// just check that the number of default replica regions in the meta table are the same
// as the number of regions the table was created with, and the count of the
// replicas is numReplica for each region
Map<RegionInfo, Integer> defaultReplicas = new HashMap<>();
for (RegionInfo hri : hris) {
Integer i;
RegionInfo regionReplica0 = RegionReplicaUtil.getRegionInfoForDefaultReplica(hri);
defaultReplicas.put(regionReplica0,
(i = defaultReplicas.get(regionReplica0)) == null ? 1 : i + 1);
Integer i = defaultReplicas.get(regionReplica0);
defaultReplicas.put(regionReplica0, i == null ? 1 : i + 1);
}
assert(defaultReplicas.size() == numRegions);
assertEquals(numRegions, defaultReplicas.size());
Collection<Integer> counts = new HashSet<>(defaultReplicas.values());
assert(counts.size() == 1 && counts.contains(numReplica));
assertEquals(1, counts.size());
assertTrue(counts.contains(numReplica));
} finally {
ADMIN.disableTable(tableName);
ADMIN.deleteTable(tableName);
}
}

@Test @Ignore("Enable when we have support for alter_table- HBASE-10361")
@Test
@Ignore("Enable when we have support for alter_table- HBASE-10361")

@petersomogyi petersomogyi Jun 9, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated, but HBASE-10361 has merged ages ago. I will open a JIRA to remove @ignore.

@Apache9 Apache9 Jun 9, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me include this in the new patch, just a one line change.

@Apache9 Apache9 Jun 9, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh the test fails locally for me... Then let's use a new issue to address it...

@petersomogyi petersomogyi Jun 9, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also fails for me and the try block catches the underlying exception from ADMIN.enableTable(tableName);. Better to work on this on a new issue.

public void testIncompleteMetaTableReplicaInformation() throws Exception {
final TableName tableName = TableName.valueOf(name.getMethodName());
final int numRegions = 3;
final int numReplica = 2;
try {
// Create a table and let the meta table be updated with the location of the
// region locations.
HTableDescriptor desc = new HTableDescriptor(tableName);
desc.setRegionReplication(numReplica);
desc.addFamily(new HColumnDescriptor("family"));
TableDescriptor desc =
TableDescriptorBuilder.newBuilder(tableName).setRegionReplication(numReplica)
.setColumnFamily(ColumnFamilyDescriptorBuilder.of("family")).build();
ADMIN.createTable(desc, Bytes.toBytes("A"), Bytes.toBytes("Z"), numRegions);
TEST_UTIL.waitTableEnabled(tableName);
TEST_UTIL.waitUntilAllRegionsAssigned(tableName);
TEST_UTIL.waitUntilNoRegionsInTransition();
Set<byte[]> tableRows = new HashSet<>();
List<RegionInfo> hris = MetaTableAccessor.getTableRegions(ADMIN.getConnection(), tableName);
for (RegionInfo hri : hris) {
Expand All @@ -295,27 +313,21 @@ public void testIncompleteMetaTableReplicaInformation() throws Exception {
// even if the meta table is partly updated, when we re-enable the table, we should
// get back the desired number of replicas for the regions
ADMIN.enableTable(tableName);
assert(ADMIN.isTableEnabled(tableName));
List<RegionInfo> regions = TEST_UTIL.getMiniHBaseCluster().getMaster()
.getAssignmentManager().getRegionStates().getRegionsOfTable(tableName);
assert(regions.size() == numRegions * numReplica);
assertTrue(ADMIN.isTableEnabled(tableName));
TEST_UTIL.waitUntilAllRegionsAssigned(tableName);
TEST_UTIL.waitUntilNoRegionsInTransition();
List<RegionInfo> regions = TEST_UTIL.getMiniHBaseCluster().getMaster().getAssignmentManager()
.getRegionStates().getRegionsOfTable(tableName);
assertEquals(numRegions * numReplica, regions.size());
} finally {
ADMIN.disableTable(tableName);
ADMIN.deleteTable(tableName);
}
}

private String printRegions(List<RegionInfo> regions) {
StringBuilder strBuf = new StringBuilder();
for (RegionInfo r : regions) {
strBuf.append(" ____ " + r.toString());
}
return strBuf.toString();
}

private void validateNumberOfRowsInMeta(final TableName table, int numRegions,
Connection connection) throws IOException {
assert(ADMIN.tableExists(table));
assert (ADMIN.tableExists(table));
final AtomicInteger count = new AtomicInteger();
Visitor visitor = new Visitor() {
@Override
Expand All @@ -325,16 +337,16 @@ public boolean visit(Result r) throws IOException {
}
};
MetaTableAccessor.fullScanRegions(connection, visitor);
assert(count.get() == numRegions);
assertEquals(numRegions, count.get());
}

private void validateFromSnapshotFromMeta(HBaseTestingUtility util, TableName table,
int numRegions, int numReplica, Connection connection) throws IOException {
SnapshotOfRegionAssignmentFromMeta snapshot = new SnapshotOfRegionAssignmentFromMeta(
connection);
SnapshotOfRegionAssignmentFromMeta snapshot =
new SnapshotOfRegionAssignmentFromMeta(connection);
snapshot.initialize();
Map<RegionInfo, ServerName> regionToServerMap = snapshot.getRegionToRegionServerMap();
assert(regionToServerMap.size() == numRegions * numReplica);
assert (regionToServerMap.size() == numRegions * numReplica);
Map<ServerName, List<RegionInfo>> serverToRegionMap = snapshot.getRegionServerToRegionMap();
for (Map.Entry<ServerName, List<RegionInfo>> entry : serverToRegionMap.entrySet()) {
if (entry.getKey().equals(util.getHBaseCluster().getMaster().getServerName())) {
Expand All @@ -345,7 +357,7 @@ private void validateFromSnapshotFromMeta(HBaseTestingUtility util, TableName ta
for (RegionInfo region : regions) {
byte[] startKey = region.getStartKey();
if (region.getTable().equals(table)) {
setOfStartKeys.add(startKey); //ignore other tables
setOfStartKeys.add(startKey); // ignore other tables
LOG.info("--STARTKEY {}--", new String(startKey, StandardCharsets.UTF_8));
}
}
Expand All @@ -357,10 +369,10 @@ private void validateFromSnapshotFromMeta(HBaseTestingUtility util, TableName ta

private void validateSingleRegionServerAssignment(Connection connection, int numRegions,
int numReplica) throws IOException {
SnapshotOfRegionAssignmentFromMeta snapshot = new SnapshotOfRegionAssignmentFromMeta(
connection);
SnapshotOfRegionAssignmentFromMeta snapshot =
new SnapshotOfRegionAssignmentFromMeta(connection);
snapshot.initialize();
Map<RegionInfo, ServerName> regionToServerMap = snapshot.getRegionToRegionServerMap();
Map<RegionInfo, ServerName> regionToServerMap = snapshot.getRegionToRegionServerMap();
assertEquals(regionToServerMap.size(), numRegions * numReplica);
Map<ServerName, List<RegionInfo>> serverToRegionMap = snapshot.getRegionServerToRegionMap();
assertEquals("One Region Only", 1, serverToRegionMap.keySet().size());
Expand Down