From 0d7a6b9725154d5b3285c9ae38f62309db5db80b Mon Sep 17 00:00:00 2001 From: Sandeep Pal <50725353+sandeepvinayak@users.noreply.github.com> Date: Tue, 26 Nov 2019 10:10:01 -0800 Subject: [PATCH] HBASE-23117: Bad enum in hbase:meta info:state column can fail loadMeta and stop startup (#867) * Handling the BAD value in info:state columns in hbase:meta * Adding a unit test and region encoded name in the LOG * Adding a null check for region state to complete the test scenario and fixing the nit Signed-off-by: Wellington Chevreuil Signed-off-by: GuangxuCheng Signed-off-by: stack --- .../master/assignment/RegionStateStore.java | 17 ++++++-- .../hadoop/hbase/HBaseTestingUtility.java | 2 +- .../assignment/TestRegionStateStore.java | 39 +++++++++++++++++++ 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java index 69bc8f70aa56..2dbd4e6476cb 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java @@ -132,7 +132,7 @@ private void visitMetaEntry(final RegionStateVisitor visitor, final Result resul if (regionInfo == null) continue; final int replicaId = regionInfo.getReplicaId(); - final State state = getRegionState(result, replicaId); + final State state = getRegionState(result, replicaId, regionInfo); final ServerName lastHost = hrl.getServerName(); final ServerName regionLocation = getRegionServer(result, replicaId); @@ -347,13 +347,22 @@ private static byte[] getServerNameColumn(int replicaId) { * @return the region state, or null if unknown. */ @VisibleForTesting - public static State getRegionState(final Result r, int replicaId) { + public static State getRegionState(final Result r, int replicaId, RegionInfo regionInfo) { Cell cell = r.getColumnLatestCell(HConstants.CATALOG_FAMILY, getStateColumn(replicaId)); if (cell == null || cell.getValueLength() == 0) { return null; } - return State.valueOf(Bytes.toString(cell.getValueArray(), cell.getValueOffset(), - cell.getValueLength())); + + String state = Bytes.toString(cell.getValueArray(), cell.getValueOffset(), + cell.getValueLength()); + try { + return State.valueOf(state); + } catch (IllegalArgumentException e) { + LOG.warn("BAD value {} in hbase:meta info:state column for region {} , " + + "Consider using HBCK2 setRegionState ENCODED_REGION_NAME STATE", + state, regionInfo.getEncodedName()); + return null; + } } private static byte[] getStateColumn(int replicaId) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java index a50ac11db6eb..29bddb0fefa0 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java @@ -3606,7 +3606,7 @@ public boolean evaluate() throws IOException { } } if (RegionStateStore.getRegionState(r, - info.getReplicaId()) != RegionState.State.OPEN) { + info.getReplicaId(), info) != RegionState.State.OPEN) { return false; } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionStateStore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionStateStore.java index b4af3708f7c6..c817b8956008 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionStateStore.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionStateStore.java @@ -19,20 +19,26 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.Result; +import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.master.RegionState; import org.apache.hadoop.hbase.regionserver.HRegion; import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.junit.After; import org.junit.Before; import org.junit.ClassRule; @@ -84,6 +90,39 @@ public void visitRegionState(Result result, RegionInfo regionInfo, RegionState.S assertTrue("Visitor has not been called.", visitorCalled.get()); } + @Test + public void testVisitMetaForBadRegionState() throws Exception { + final TableName tableName = TableName.valueOf("testVisitMetaForBadRegionState"); + util.createTable(tableName, "cf"); + final List regions = util.getHBaseCluster().getRegions(tableName); + final String encodedName = regions.get(0).getRegionInfo().getEncodedName(); + final RegionStateStore regionStateStore = util.getHBaseCluster().getMaster(). + getAssignmentManager().getRegionStateStore(); + + // add the BAD_STATE which does not exist in enum RegionState.State + Put put = new Put(regions.get(0).getRegionInfo().getRegionName(), + EnvironmentEdgeManager.currentTime()); + put.addColumn(HConstants.CATALOG_FAMILY, HConstants.STATE_QUALIFIER, + Bytes.toBytes("BAD_STATE")); + + try (Table table = util.getConnection().getTable(TableName.META_TABLE_NAME)) { + table.put(put); + } + + final AtomicBoolean visitorCalled = new AtomicBoolean(false); + regionStateStore.visitMetaForRegion(encodedName, new RegionStateStore.RegionStateVisitor() { + @Override + public void visitRegionState(Result result, RegionInfo regionInfo, + RegionState.State state, ServerName regionLocation, + ServerName lastHost, long openSeqNum) { + assertEquals(encodedName, regionInfo.getEncodedName()); + assertNull(state); + visitorCalled.set(true); + } + }); + assertTrue("Visitor has not been called.", visitorCalled.get()); + } + @Test public void testVisitMetaForRegionNonExistingRegion() throws Exception { final String encodedName = "fakeencodedregionname";