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 2024/04/07 10:35:23 UTC
(hbase) branch branch-2.5 updated (afd4da05914 -> 33fe2047837)
This is an automated email from the ASF dual-hosted git repository.
zhangduo pushed a change to branch branch-2.5
in repository https://gitbox.apache.org/repos/asf/hbase.git
from afd4da05914 HBASE-28366 Mis-order of SCP and regionServerReport results into region inconsistencies (#5774)
new 479c756291f HBASE-28457 Introduce a version field in file based tracker record (#5784)
new 33fe2047837 HBASE-28481 Prompting table already exists after failing to create table with many region replications (#5789)
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../hadoop/hbase/client/MutableRegionInfo.java | 2 +-
.../src/main/protobuf/StoreFileTracker.proto | 1 +
.../master/procedure/CreateTableProcedure.java | 13 ++++-
.../storefiletracker/StoreFileListFile.java | 57 ++++++++++++++++------
.../master/procedure/TestCreateTableProcedure.java | 29 +++++++++++
.../storefiletracker/TestStoreFileListFile.java | 16 ++++++
6 files changed, 101 insertions(+), 17 deletions(-)
(hbase) 02/02: HBASE-28481 Prompting table already exists after failing to create table with many region replications (#5789)
Posted by zh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zhangduo pushed a commit to branch branch-2.5
in repository https://gitbox.apache.org/repos/asf/hbase.git
commit 33fe2047837f60f91ed509ec1e285cfe5633407e
Author: guluo <lu...@qq.com>
AuthorDate: Sun Apr 7 17:27:28 2024 +0800
HBASE-28481 Prompting table already exists after failing to create table with many region replications (#5789)
Signed-off-by: Duo Zhang <zh...@apache.org>
Reviewed-by: Vineet Kumar Maheshwari <vi...@gmail.com>
(cherry picked from commit e5d59cadc5dc9ac7d6f15555be1a8defc05862ad)
---
.../hadoop/hbase/client/MutableRegionInfo.java | 2 +-
.../master/procedure/CreateTableProcedure.java | 13 ++++++++--
.../master/procedure/TestCreateTableProcedure.java | 29 ++++++++++++++++++++++
3 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MutableRegionInfo.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MutableRegionInfo.java
index a9382f3a9be..4217201b85e 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MutableRegionInfo.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MutableRegionInfo.java
@@ -96,7 +96,7 @@ class MutableRegionInfo implements RegionInfo {
private static int checkReplicaId(int regionId) {
if (regionId > MAX_REPLICA_ID) {
- throw new IllegalArgumentException("ReplicaId cannot be greater than" + MAX_REPLICA_ID);
+ throw new IllegalArgumentException("ReplicaId cannot be greater than " + MAX_REPLICA_ID);
}
return regionId;
}
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java
index 37aea5e2f7d..08be7238821 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java
@@ -55,6 +55,8 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos.C
public class CreateTableProcedure extends AbstractStateMachineTableProcedure<CreateTableState> {
private static final Logger LOG = LoggerFactory.getLogger(CreateTableProcedure.class);
+ private static final int MAX_REGION_REPLICATION = 0x10000;
+
private TableDescriptor tableDescriptor;
private List<RegionInfo> newRegions;
@@ -83,10 +85,10 @@ public class CreateTableProcedure extends AbstractStateMachineTableProcedure<Cre
switch (state) {
case CREATE_TABLE_PRE_OPERATION:
// Verify if we can create the table
- boolean exists = !prepareCreate(env);
+ boolean success = prepareCreate(env);
releaseSyncLatch();
- if (exists) {
+ if (!success) {
assert isFailed() : "the delete should have an exception here";
return Flow.NO_MORE_STATE;
}
@@ -252,6 +254,13 @@ public class CreateTableProcedure extends AbstractStateMachineTableProcedure<Cre
return false;
}
+ int regionReplicationCount = tableDescriptor.getRegionReplication();
+ if (regionReplicationCount > MAX_REGION_REPLICATION) {
+ setFailure("master-create-table", new IllegalArgumentException(
+ "Region Replication cannot exceed " + MAX_REGION_REPLICATION + "."));
+ return false;
+ }
+
// check for store file tracker configurations
StoreFileTrackerValidationUtils.checkForCreateTable(env.getMasterConfiguration(),
tableDescriptor);
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java
index 618a4a45a04..bed41f4da86 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java
@@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.master.procedure;
import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -284,4 +285,32 @@ public class TestCreateTableProcedure extends TestTableDDLProcedureBase {
new CreateTableProcedureOnHDFSFailure(procExec.getEnvironment(), htd, regions));
ProcedureTestingUtility.assertProcNotFailed(procExec, procId);
}
+
+ @Test
+ public void testCreateTableWithManyRegionReplication() throws IOException {
+ final int EXCEED_MAX_REGION_REPLICATION = 0x10001;
+ TableName tableName = TableName.valueOf(name.getMethodName());
+ ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
+
+ TableDescriptor tableWithManyRegionReplication = TableDescriptorBuilder.newBuilder(tableName)
+ .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(Bytes.toBytes("f1")).build())
+ .setRegionReplication(EXCEED_MAX_REGION_REPLICATION).build();
+ RegionInfo[] regions01 =
+ ModifyRegionUtils.createRegionInfos(tableWithManyRegionReplication, null);
+ long procId01 = ProcedureTestingUtility.submitAndWait(procExec, new CreateTableProcedure(
+ procExec.getEnvironment(), tableWithManyRegionReplication, regions01));
+ Procedure<?> result01 = procExec.getResult(procId01);
+ assertTrue(result01.getException().getCause() instanceof IllegalArgumentException);
+ assertFalse(UTIL.getAdmin().tableExists(tableName));
+
+ TableDescriptor tdesc = TableDescriptorBuilder.newBuilder(tableName)
+ .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(Bytes.toBytes("f1")).build())
+ .build();
+ RegionInfo[] regions02 = ModifyRegionUtils.createRegionInfos(tdesc, null);
+ long procId02 = ProcedureTestingUtility.submitAndWait(procExec,
+ new CreateTableProcedure(procExec.getEnvironment(), tdesc, regions02));
+ Procedure<?> result02 = procExec.getResult(procId02);
+ assertTrue(result02.isSuccess());
+ assertTrue(UTIL.getAdmin().tableExists(tableName));
+ }
}
(hbase) 01/02: HBASE-28457 Introduce a version field in file based tracker record (#5784)
Posted by zh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zhangduo pushed a commit to branch branch-2.5
in repository https://gitbox.apache.org/repos/asf/hbase.git
commit 479c756291f27b0b5c004ee45098948140a8fc6e
Author: Duo Zhang <zh...@apache.org>
AuthorDate: Sun Apr 7 16:43:50 2024 +0800
HBASE-28457 Introduce a version field in file based tracker record (#5784)
Signed-off-by: Wellington Chevreuil <wc...@apache.org>
(cherry picked from commit c1012a9ebec9bb9fcc09f2d6fdc78e74cc44d562)
---
.../src/main/protobuf/StoreFileTracker.proto | 1 +
.../storefiletracker/StoreFileListFile.java | 57 ++++++++++++++++------
.../storefiletracker/TestStoreFileListFile.java | 16 ++++++
3 files changed, 60 insertions(+), 14 deletions(-)
diff --git a/hbase-protocol-shaded/src/main/protobuf/StoreFileTracker.proto b/hbase-protocol-shaded/src/main/protobuf/StoreFileTracker.proto
index 2a269ea4ac4..001cb3ea233 100644
--- a/hbase-protocol-shaded/src/main/protobuf/StoreFileTracker.proto
+++ b/hbase-protocol-shaded/src/main/protobuf/StoreFileTracker.proto
@@ -33,4 +33,5 @@ message StoreFileEntry {
message StoreFileList {
required uint64 timestamp = 1;
repeated StoreFileEntry store_file = 2;
+ optional uint64 version = 3 [default = 1];
}
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileListFile.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileListFile.java
index c8cf55aeba6..48a38038914 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileListFile.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileListFile.java
@@ -17,6 +17,7 @@
*/
package org.apache.hadoop.hbase.regionserver.storefiletracker;
+import com.google.errorprone.annotations.RestrictedApi;
import java.io.EOFException;
import java.io.FileNotFoundException;
import java.io.IOException;
@@ -47,15 +48,24 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.StoreFileTrackerProtos.
* without error on partial bytes if you stop at some special points, but the return message will
* have incorrect field value. We should try our best to prevent this happens because loading an
* incorrect store file list file usually leads to data loss.
+ * <p/>
+ * To prevent failing silently while downgrading, where we may miss some newly introduced fields in
+ * {@link StoreFileList} which are necessary, we introduce a 'version' field in
+ * {@link StoreFileList}. If we find out that we are reading a {@link StoreFileList} with higher
+ * version, we will fail immediately and tell users that you need extra steps while downgrading, to
+ * prevent potential data loss.
*/
@InterfaceAudience.Private
class StoreFileListFile {
private static final Logger LOG = LoggerFactory.getLogger(StoreFileListFile.class);
+ // the current version for StoreFileList
+ static final long VERSION = 1;
+
static final String TRACK_FILE_DIR = ".filelist";
- private static final String TRACK_FILE = "f1";
+ static final String TRACK_FILE = "f1";
private static final String TRACK_FILE_ROTATE = "f2";
@@ -101,7 +111,18 @@ class StoreFileListFile {
throw new IOException(
"Checksum mismatch, expected " + expectedChecksum + ", actual " + calculatedChecksum);
}
- return StoreFileList.parseFrom(data);
+ StoreFileList storeFileList = StoreFileList.parseFrom(data);
+ if (storeFileList.getVersion() > VERSION) {
+ LOG.error(
+ "The loaded store file list is in version {}, which is higher than expected"
+ + " version {}. Stop loading to prevent potential data loss. This usually because your"
+ + " cluster is downgraded from a newer version. You need extra steps before downgrading,"
+ + " like switching back to default store file tracker.",
+ storeFileList.getVersion(), VERSION);
+ throw new IOException("Higher store file list version detected, expected " + VERSION
+ + ", got " + storeFileList.getVersion());
+ }
+ return storeFileList;
}
private int select(StoreFileList[] lists) {
@@ -134,30 +155,38 @@ class StoreFileListFile {
return lists[winnerIndex];
}
+ @RestrictedApi(explanation = "Should only be called in tests", link = "",
+ allowedOnPath = ".*/StoreFileListFile.java|.*/src/test/.*")
+ static void write(FileSystem fs, Path file, StoreFileList storeFileList) throws IOException {
+ byte[] data = storeFileList.toByteArray();
+ CRC32 crc32 = new CRC32();
+ crc32.update(data);
+ int checksum = (int) crc32.getValue();
+ // 4 bytes length at the beginning, plus 4 bytes checksum
+ try (FSDataOutputStream out = fs.create(file, true)) {
+ out.writeInt(data.length);
+ out.write(data);
+ out.writeInt(checksum);
+ }
+ }
+
/**
- * We will set the timestamp in this method so just pass the builder in
+ * We will set the timestamp and version in this method so just pass the builder in
*/
void update(StoreFileList.Builder builder) throws IOException {
if (nextTrackFile < 0) {
// we need to call load first to load the prevTimestamp and also the next file
load();
}
- long timestamp = Math.max(prevTimestamp + 1, EnvironmentEdgeManager.currentTime());
- byte[] actualData = builder.setTimestamp(timestamp).build().toByteArray();
- CRC32 crc32 = new CRC32();
- crc32.update(actualData);
- int checksum = (int) crc32.getValue();
- // 4 bytes length at the beginning, plus 4 bytes checksum
FileSystem fs = ctx.getRegionFileSystem().getFileSystem();
- try (FSDataOutputStream out = fs.create(trackFiles[nextTrackFile], true)) {
- out.writeInt(actualData.length);
- out.write(actualData);
- out.writeInt(checksum);
- }
+ long timestamp = Math.max(prevTimestamp + 1, EnvironmentEdgeManager.currentTime());
+ write(fs, trackFiles[nextTrackFile],
+ builder.setTimestamp(timestamp).setVersion(VERSION).build());
// record timestamp
prevTimestamp = timestamp;
// rotate the file
nextTrackFile = 1 - nextTrackFile;
+
try {
fs.delete(trackFiles[nextTrackFile], false);
} catch (IOException e) {
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileListFile.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileListFile.java
index 3a90216035e..998671264ba 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileListFile.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileListFile.java
@@ -17,6 +17,7 @@
*/
package org.apache.hadoop.hbase.regionserver.storefiletracker;
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;
import static org.mockito.Mockito.mock;
@@ -35,6 +36,7 @@ import org.apache.hadoop.hbase.regionserver.StoreContext;
import org.apache.hadoop.hbase.testclassification.RegionServerTests;
import org.apache.hadoop.hbase.testclassification.SmallTests;
import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.ClassRule;
@@ -162,4 +164,18 @@ public class TestStoreFileListFile {
write(fs, trackerFileStatus.getPath(), content, 0, content.length);
assertThrows(IOException.class, () -> storeFileListFile.load());
}
+
+ @Test
+ public void testLoadHigherVersion() throws IOException {
+ // write a fake StoreFileList file with higher version
+ StoreFileList storeFileList =
+ StoreFileList.newBuilder().setVersion(StoreFileListFile.VERSION + 1)
+ .setTimestamp(EnvironmentEdgeManager.currentTime()).build();
+ Path trackFileDir = new Path(testDir, StoreFileListFile.TRACK_FILE_DIR);
+ StoreFileListFile.write(FileSystem.get(UTIL.getConfiguration()),
+ new Path(trackFileDir, StoreFileListFile.TRACK_FILE), storeFileList);
+ IOException error = assertThrows(IOException.class, () -> storeFileListFile.load());
+ assertEquals("Higher store file list version detected, expected " + StoreFileListFile.VERSION
+ + ", got " + (StoreFileListFile.VERSION + 1), error.getMessage());
+ }
}