diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java index b8e6b496995b..c5fa50c37427 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java @@ -321,7 +321,7 @@ private void prepareModify(final MasterProcedureEnv env) throws IOException { // check for store file tracker configurations StoreFileTrackerValidationUtils.checkForModifyTable(env.getMasterConfiguration(), - unmodifiedTableDescriptor, modifiedTableDescriptor); + unmodifiedTableDescriptor, modifiedTableDescriptor, !isTableEnabled(env)); } /** diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerValidationUtils.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerValidationUtils.java index e6f6e854c890..38040bc4f006 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerValidationUtils.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerValidationUtils.java @@ -20,6 +20,7 @@ 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 static void checkForCreateTable(Configuration conf, TableDescriptor table * {@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 static void checkForModifyTable(Configuration conf, TableDescriptor oldTa 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 static void checkForModifyTable(Configuration conf, TableDescriptor oldTa 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 newSrcTracker = MigrationStoreFileTracker.getSrcTrackerClass(newConf); if (!oldTracker.equals(newSrcTracker)) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestChangeStoreFileTracker.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestChangeStoreFileTracker.java index 110f896df8b8..4b54cd9131ba 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestChangeStoreFileTracker.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestChangeStoreFileTracker.java @@ -19,6 +19,7 @@ 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.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 void testModifyError8() throws IOException { 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))