You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zh...@apache.org on 2018/07/02 13:46:23 UTC
hbase git commit: HBASE-20817 Infinite loop when executing
ReopenTableRegionsProcedure
Repository: hbase
Updated Branches:
refs/heads/master 7c5188f39 -> cfdabe926
HBASE-20817 Infinite loop when executing ReopenTableRegionsProcedure
Signed-off-by: zhangduo <zh...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/cfdabe92
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/cfdabe92
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/cfdabe92
Branch: refs/heads/master
Commit: cfdabe92672b68cc74fa14b3f5bdc4b95f79538e
Parents: 7c5188f
Author: Ankit Singhal <an...@gmail.com>
Authored: Mon Jul 2 16:07:34 2018 +0800
Committer: zhangduo <zh...@apache.org>
Committed: Mon Jul 2 21:26:14 2018 +0800
----------------------------------------------------------------------
.../master/assignment/MoveRegionProcedure.java | 8 +++---
.../hadoop/hbase/regionserver/HRegion.java | 24 +++++++++--------
.../apache/hadoop/hbase/client/TestAdmin1.java | 27 +++++++++++++++++---
3 files changed, 43 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/cfdabe92/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/cfdabe92/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 7771ad0..02ae7d8 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
@@ -974,8 +974,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) {
- LOG.debug("writing seq id for " + this.getRegionInfo().getEncodedName());
+ // 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 - 1);
}
@@ -1657,7 +1660,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);
}
@@ -7050,7 +7056,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)
@@ -7072,7 +7077,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,
@@ -7096,7 +7100,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,
@@ -7121,7 +7124,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 {
@@ -7154,8 +7156,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;
@@ -7168,7 +7173,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/cfdabe92/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 e0eef20..b3df2cc 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;
@@ -69,6 +68,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;
@@ -501,9 +501,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 {