Repository: hbase Updated Branches: refs/heads/branch-2.0 4b5ecbfb9 -> 0390139f7
HBASE-20817 Infinite loop when executing ReopenTableRegionsProcedure Signed-off-by: zhangduo <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/0390139f Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/0390139f Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/0390139f Branch: refs/heads/branch-2.0 Commit: 0390139f78a85ecefd950e3fa3ad20d06c9bd42e Parents: 4b5ecbf Author: Ankit Singhal <[email protected]> Authored: Mon Jul 2 16:07:34 2018 +0800 Committer: zhangduo <[email protected]> Committed: Mon Jul 2 21:30:22 2018 +0800 ---------------------------------------------------------------------- .../master/assignment/MoveRegionProcedure.java | 8 +++--- .../hadoop/hbase/regionserver/HRegion.java | 23 ++++++++++------- .../apache/hadoop/hbase/client/TestAdmin1.java | 27 +++++++++++++++++--- 3 files changed, 43 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/0390139f/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java index 139d41d..6135ce1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java @@ -66,8 +66,10 @@ public class MoveRegionProcedure extends AbstractStateMachineRegionProcedure<Mov throws HBaseIOException { super(env, plan.getRegionInfo()); this.plan = plan; - preflightChecks(env, true); - checkOnline(env, plan.getRegionInfo()); + if (check) { + preflightChecks(env, true); + checkOnline(env, plan.getRegionInfo()); + } } @Override @@ -135,7 +137,7 @@ public class MoveRegionProcedure extends AbstractStateMachineRegionProcedure<Mov @Override protected MoveRegionState getState(final int stateId) { - return MoveRegionState.valueOf(stateId); + return MoveRegionState.forNumber(stateId); } @Override http://git-wip-us.apache.org/repos/asf/hbase/blob/0390139f/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index c748ac3..dea9803 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -950,7 +950,11 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi long maxSeqIdFromFile = WALSplitter.getMaxRegionSequenceId(fs.getFileSystem(), fs.getRegionDir()); long nextSeqId = Math.max(maxSeqId, maxSeqIdFromFile) + 1; - if (writestate.writesEnabled) { + // The openSeqNum will always be increase even for read only region, as we rely on it to + // determine whether a region has been successfully reopend, so here we always need to update + // the max sequence id file. + if (RegionReplicaUtil.isDefaultReplica(getRegionInfo())) { + LOG.debug("writing seq id for {}", this.getRegionInfo().getEncodedName()); WALSplitter.writeRegionSequenceIdFile(fs.getFileSystem(), fs.getRegionDir(), nextSeqId); } @@ -1625,7 +1629,10 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi } status.setStatus("Writing region close event to WAL"); - if (!abort && wal != null && getRegionServerServices() != null && !writestate.readOnly) { + // Always write close marker to wal even for read only table. This is not a big problem as we + // do not write any data into the region. + if (!abort && wal != null && getRegionServerServices() != null && + RegionReplicaUtil.isDefaultReplica(getRegionInfo())) { writeRegionCloseMarker(wal); } @@ -6925,7 +6932,6 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi * HRegion#getMinSequenceId() to ensure the wal id is properly kept * up. HRegionStore does this every time it opens a new region. * @return new HRegion - * @throws IOException */ public static HRegion openHRegion(final Configuration conf, final FileSystem fs, final Path rootDir, final RegionInfo info, final TableDescriptor htd, final WAL wal) @@ -6947,7 +6953,6 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi * @param rsServices An interface we can request flushes against. * @param reporter An interface we can report progress against. * @return new HRegion - * @throws IOException */ public static HRegion openHRegion(final Configuration conf, final FileSystem fs, final Path rootDir, final RegionInfo info, final TableDescriptor htd, final WAL wal, @@ -6971,7 +6976,6 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi * @param rsServices An interface we can request flushes against. * @param reporter An interface we can report progress against. * @return new HRegion - * @throws IOException */ public static HRegion openHRegion(final Configuration conf, final FileSystem fs, final Path rootDir, final Path tableDir, final RegionInfo info, final TableDescriptor htd, @@ -6996,7 +7000,6 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi * @param other original object * @param reporter An interface we can report progress against. * @return new HRegion - * @throws IOException */ public static HRegion openHRegion(final HRegion other, final CancelableProgressable reporter) throws IOException { @@ -7027,8 +7030,11 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi checkClassLoading(); this.openSeqNum = initialize(reporter); this.mvcc.advanceTo(openSeqNum); - if (wal != null && getRegionServerServices() != null && !writestate.readOnly) { - // Only write the region open event marker to WAL if we are not read-only. + // The openSeqNum must be increased every time when a region is assigned, as we rely on it to + // determine whether a region has been successfully reopened. So here we always write open + // marker, even if the table is read only. + if (wal != null && getRegionServerServices() != null && + RegionReplicaUtil.isDefaultReplica(getRegionInfo())) { writeRegionOpenMarker(wal, openSeqNum); } return this; @@ -7041,7 +7047,6 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi * @param info Info for region to be opened. * @param htd the table descriptor * @return new HRegion - * @throws IOException e */ public static HRegion openReadOnlyFileSystemHRegion(final Configuration conf, final FileSystem fs, final Path tableDir, RegionInfo info, final TableDescriptor htd) throws IOException { http://git-wip-us.apache.org/repos/asf/hbase/blob/0390139f/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java index 8ac2ddaf..a9a15aa 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java @@ -31,7 +31,6 @@ import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; - import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HColumnDescriptor; @@ -68,6 +67,7 @@ import org.junit.experimental.categories.Category; import org.junit.rules.TestName; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + import org.apache.hadoop.hbase.shaded.protobuf.RequestConverter; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.MergeTableRegionsRequest; @@ -500,9 +500,30 @@ public class TestAdmin1 { } /** + * Verify schema change for read only table + */ + @Test + public void testReadOnlyTableModify() throws IOException, InterruptedException { + final TableName tableName = TableName.valueOf(name.getMethodName()); + TEST_UTIL.createTable(tableName, HConstants.CATALOG_FAMILY).close(); + + // Make table read only + TableDescriptor htd = TableDescriptorBuilder.newBuilder(this.admin.getDescriptor(tableName)) + .setReadOnly(true).build(); + admin.modifyTable(htd); + + // try to modify the read only table now + htd = TableDescriptorBuilder.newBuilder(this.admin.getDescriptor(tableName)) + .setCompactionEnabled(false).build(); + admin.modifyTable(htd); + // Delete the table + this.admin.disableTable(tableName); + this.admin.deleteTable(tableName); + assertFalse(this.admin.tableExists(tableName)); + } + + /** * Verify schema modification takes. - * @throws IOException - * @throws InterruptedException */ @Test public void testOnlineChangeTableSchema() throws IOException, InterruptedException {
