Apache
/
hbase
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

HBASE-26611 Changing SFT implementation on disabled table is dangerous (#4082) 

Signed-off-by: Xiaolin Ha <haxiaolin@apache.org>
This commit is contained in:
Duo Zhang 2022-03-15 20:13:46 +08:00 committed by GitHub
parent 5ad51e8b2b
commit a816204793
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 38 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java
View File

@ -327,7 +327,7 @@ public class ModifyTableProcedure
// check for store file tracker configurations
StoreFileTrackerValidationUtils.checkForModifyTable(env.getMasterConfiguration(),
unmodifiedTableDescriptor, modifiedTableDescriptor);
unmodifiedTableDescriptor, modifiedTableDescriptor, !isTableEnabled(env));
}
/**

16
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerValidationUtils.java
View File

@ -20,6 +20,7 @@ package org.apache.hadoop.hbase.regionserver.storefiletracker;
import java.io.IOException;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.DoNotRetryIOException;
import org.apache.hadoop.hbase.TableNotEnabledException;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.regionserver.StoreUtils;
@ -94,7 +95,7 @@ public final class StoreFileTrackerValidationUtils {
* {@code ModifyTableProcedure}.
*/
public static void checkForModifyTable(Configuration conf, TableDescriptor oldTable,
TableDescriptor newTable) throws IOException {
TableDescriptor newTable, boolean isTableDisabled) throws IOException {
for (ColumnFamilyDescriptor newFamily : newTable.getColumnFamilies()) {
ColumnFamilyDescriptor oldFamily = oldTable.getColumnFamily(newFamily.getName());
if (oldFamily == null) {
@ -133,6 +134,16 @@ public final class StoreFileTrackerValidationUtils {
newFamily.getNameAsString() + " of table " + newTable.getTableName());
}
} else {
// do not allow changing from MIGRATION to its dst SFT implementation while the table is
// disabled. We need to open the HRegion to migrate the tracking information while the SFT
// implementation is MIGRATION, otherwise we may loss data. See HBASE-26611 for more
// details.
if (isTableDisabled) {
throw new TableNotEnabledException(
"Should not change store file tracker implementation from " +
StoreFileTrackerFactory.Trackers.MIGRATION.name() + " while table " +
newTable.getTableName() + " is disabled");
}
// we can only change to the dst tracker
if (!newTracker.equals(oldDstTracker)) {
throw new DoNotRetryIOException("Should migrate tracker to " +
@ -151,6 +162,9 @@ public final class StoreFileTrackerValidationUtils {
StoreFileTrackerFactory.getStoreFileTrackerName(oldTracker) + " for family " +
newFamily.getNameAsString() + " of table " + newTable.getTableName());
}
// here we do not check whether the table is disabled, as after changing to MIGRATION, we
// still rely on the src SFT implementation to actually load the store files, so there
// will be no data loss problem.
Class<? extends StoreFileTracker> newSrcTracker =
MigrationStoreFileTracker.getSrcTrackerClass(newConf);
if (!oldTracker.equals(newSrcTracker)) {

22
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestChangeStoreFileTracker.java
View File

@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.regionserver.storefiletracker;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
import java.io.IOException;
import org.apache.hadoop.hbase.DoNotRetryIOException;
@ -26,6 +27,7 @@ import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HBaseTestingUtil;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.TableNameTestRule;
import org.apache.hadoop.hbase.TableNotEnabledException;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
import org.apache.hadoop.hbase.client.Get;
import org.apache.hadoop.hbase.client.Put;
@ -195,6 +197,26 @@ public class TestChangeStoreFileTracker {
UTIL.getAdmin().modifyTable(newTd);
}
@Test
public void testModifyError9() throws IOException {
TableDescriptor td = TableDescriptorBuilder.newBuilder(tableName.getTableName())
.setColumnFamily(ColumnFamilyDescriptorBuilder.of("family")).build();
UTIL.getAdmin().createTable(td);
UTIL.getAdmin().disableTable(td.getTableName());
TableDescriptor newTd = TableDescriptorBuilder.newBuilder(td)
.setValue(StoreFileTrackerFactory.TRACKER_IMPL,
StoreFileTrackerFactory.Trackers.MIGRATION.name())
.setValue(MigrationStoreFileTracker.SRC_IMPL, StoreFileTrackerFactory.Trackers.DEFAULT.name())
.setValue(MigrationStoreFileTracker.DST_IMPL, StoreFileTrackerFactory.Trackers.FILE.name())
.build();
UTIL.getAdmin().modifyTable(newTd);
TableDescriptor newTd2 = TableDescriptorBuilder.newBuilder(td)
.setValue(StoreFileTrackerFactory.TRACKER_IMPL, StoreFileTrackerFactory.Trackers.FILE.name())
.build();
// changing from MIGRATION while table is disabled is not allowed
assertThrows(TableNotEnabledException.class, () -> UTIL.getAdmin().modifyTable(newTd2));
}
private String getStoreFileName(TableName table, byte[] family) {
return Iterables
.getOnlyElement(Iterables.getOnlyElement(UTIL.getMiniHBaseCluster().getRegions(table))