HBASE-26611 Changing SFT implementation on disabled table is dangerous (#4082)
Signed-off-by: Xiaolin Ha <haxiaolin@apache.org>
This commit is contained in:
parent
350db514c7
commit
a4b192e33d
|
@ -321,7 +321,7 @@ public class ModifyTableProcedure
|
|||
|
||||
// check for store file tracker configurations
|
||||
StoreFileTrackerValidationUtils.checkForModifyTable(env.getMasterConfiguration(),
|
||||
unmodifiedTableDescriptor, modifiedTableDescriptor);
|
||||
unmodifiedTableDescriptor, modifiedTableDescriptor, !isTableEnabled(env));
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -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)) {
|
||||
|
|
|
@ -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.HBaseTestingUtility;
|
||||
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))
|
||||
|
|
Loading…
Reference in New Issue