From f120602f1e3ac2260bce9d7925f01e79bbf0f6df Mon Sep 17 00:00:00 2001 From: tedyu Date: Tue, 15 Mar 2016 16:15:03 -0700 Subject: [PATCH] HBASE-15456 CreateTableProcedure/ModifyTableProcedure needs to fail when there is no family in table descriptor (huaxiang sun) --- .../procedure/CreateTableProcedure.java | 9 +++++++ .../procedure/ModifyTableProcedure.java | 7 ++++++ .../hadoop/hbase/regionserver/HRegion.java | 7 ++++++ .../procedure/TestCreateTableProcedure.java | 24 ++++++++++++++++++ .../procedure/TestModifyTableProcedure.java | 24 ++++++++++++++++-- .../hbase/namespace/TestNamespaceAuditor.java | 25 +++++++++++++++++++ .../hadoop/hbase/util/TestRegionMover.java | 4 +++ 7 files changed, 98 insertions(+), 2 deletions(-) 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 183871d429b..8ce83351ab6 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 @@ -31,6 +31,7 @@ import org.apache.commons.logging.LogFactory; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.MetaTableAccessor; @@ -283,6 +284,14 @@ public class CreateTableProcedure setFailure("master-create-table", new TableExistsException(getTableName())); return false; } + + // check that we have at least 1 CF + if (hTableDescriptor.getColumnFamilies().length == 0) { + setFailure("master-create-table", new DoNotRetryIOException("Table " + + getTableName().toString() + " should have at least one column family.")); + return false; + } + return true; } 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 923163907db..bd1451ad026 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 @@ -29,6 +29,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.HTableDescriptor; @@ -285,6 +286,12 @@ public class ModifyTableProcedure throw new TableNotFoundException(getTableName()); } + // check that we have at least 1 CF + if (modifiedHTableDescriptor.getColumnFamilies().length == 0) { + throw new DoNotRetryIOException("Table " + getTableName().toString() + + " should have at least one column family."); + } + // In order to update the descriptor, we need to retrieve the old descriptor for comparison. this.unmodifiedHTableDescriptor = env.getMasterServices().getTableDescriptors().get(getTableName()); 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 ed91234f91f..c44145e8175 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 @@ -785,6 +785,13 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi * @throws IOException e */ private long initialize(final CancelableProgressable reporter) throws IOException { + + //Refuse to open the region if there is no column family in the table + if (htableDescriptor.getColumnFamilies().length == 0) { + throw new DoNotRetryIOException("Table " + htableDescriptor.getNameAsString() + + " should have at least one column family."); + } + MonitoredTask status = TaskMonitor.get().createStatus("Initializing region " + this); long nextSeqId = -1; try { 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 676a3f4dec9..8f6e11abc45 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 @@ -23,10 +23,12 @@ import java.io.IOException; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.HTableDescriptor; +import org.apache.hadoop.hbase.ProcedureInfo; import org.apache.hadoop.hbase.TableExistsException; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; @@ -43,6 +45,7 @@ import org.junit.BeforeClass; import org.junit.Test; import org.junit.experimental.categories.Category; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @Category({MasterTests.class, MediumTests.class}) @@ -119,6 +122,27 @@ public class TestCreateTableProcedure { UTIL.getHBaseCluster().getMaster(), tableName, regions, "f1", "f2"); } + @Test(timeout=60000) + public void testCreateWithoutColumnFamily() throws Exception { + final ProcedureExecutor procExec = getMasterProcedureExecutor(); + final TableName tableName = TableName.valueOf("testCreateWithoutColumnFamily"); + // create table with 0 families will fail + final HTableDescriptor htd = MasterProcedureTestingUtility.createHTD(tableName); + + // disable sanity check + htd.setConfiguration("hbase.table.sanity.checks", Boolean.FALSE.toString()); + final HRegionInfo[] regions = ModifyRegionUtils.createHRegionInfos(htd, null); + + long procId = + ProcedureTestingUtility.submitAndWait(procExec, + new CreateTableProcedure(procExec.getEnvironment(), htd, regions)); + final ProcedureInfo result = procExec.getResult(procId); + assertEquals(true, result.isFailed()); + Throwable cause = ProcedureTestingUtility.getExceptionCause(result); + assertTrue("expected DoNotRetryIOException, got " + cause, + cause instanceof DoNotRetryIOException); + } + @Test(timeout=60000, expected=TableExistsException.class) public void testCreateExisting() throws Exception { final TableName tableName = TableName.valueOf("testCreateExisting"); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java index ebe70d29a9f..ab86eda391a 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java @@ -25,11 +25,13 @@ import static org.junit.Assert.assertTrue; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.HTableDescriptor; +import org.apache.hadoop.hbase.ProcedureInfo; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility; @@ -168,12 +170,13 @@ public class TestModifyTableProcedure { @Test(timeout = 60000) public void testModifyTableDeleteCF() throws Exception { - final TableName tableName = TableName.valueOf("testModifyTableAddCF"); + final TableName tableName = TableName.valueOf("testModifyTableDeleteCF"); + final String cf1 = "cf1"; final String cf2 = "cf2"; final String cf3 = "cf3"; final ProcedureExecutor procExec = getMasterProcedureExecutor(); - MasterProcedureTestingUtility.createTable(procExec, tableName, null, "cf1", cf2, cf3); + MasterProcedureTestingUtility.createTable(procExec, tableName, null, cf1, cf2, cf3); HTableDescriptor currentHtd = UTIL.getHBaseAdmin().getTableDescriptor(tableName); assertEquals(3, currentHtd.getFamiliesKeys().size()); @@ -196,6 +199,8 @@ public class TestModifyTableProcedure { HTableDescriptor htd2 = new HTableDescriptor(UTIL.getHBaseAdmin().getTableDescriptor(tableName)); htd2.removeFamily(cf3.getBytes()); + // Disable Sanity check + htd2.setConfiguration("hbase.table.sanity.checks", Boolean.FALSE.toString()); long procId2 = ProcedureTestingUtility.submitAndWait(procExec, @@ -205,6 +210,21 @@ public class TestModifyTableProcedure { currentHtd = UTIL.getHBaseAdmin().getTableDescriptor(tableName); assertEquals(1, currentHtd.getFamiliesKeys().size()); assertFalse(currentHtd.hasFamily(cf3.getBytes())); + + //Removing the last family will fail + HTableDescriptor htd3 = + new HTableDescriptor(UTIL.getHBaseAdmin().getTableDescriptor(tableName)); + htd3.removeFamily(cf1.getBytes()); + long procId3 = + ProcedureTestingUtility.submitAndWait(procExec, + new ModifyTableProcedure(procExec.getEnvironment(), htd3)); + final ProcedureInfo result = procExec.getResult(procId3); + assertEquals(true, result.isFailed()); + Throwable cause = ProcedureTestingUtility.getExceptionCause(result); + assertTrue("expected DoNotRetryIOException, got " + cause, + cause instanceof DoNotRetryIOException); + assertEquals(1, currentHtd.getFamiliesKeys().size()); + assertTrue(currentHtd.hasFamily(cf1.getBytes())); } @Test(timeout=60000) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/namespace/TestNamespaceAuditor.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/namespace/TestNamespaceAuditor.java index e3b841bd3b5..78fde218da0 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/namespace/TestNamespaceAuditor.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/namespace/TestNamespaceAuditor.java @@ -148,12 +148,17 @@ public class TestNamespaceAuditor { ADMIN.createNamespace(nspDesc); assertNotNull("Namespace descriptor found null.", ADMIN.getNamespaceDescriptor(nsp)); assertEquals(ADMIN.listNamespaceDescriptors().length, 3); + HColumnDescriptor fam1 = new HColumnDescriptor("fam1"); + HTableDescriptor tableDescOne = new HTableDescriptor(TableName.valueOf(nsp + TableName.NAMESPACE_DELIM + "table1")); + tableDescOne.addFamily(fam1); HTableDescriptor tableDescTwo = new HTableDescriptor(TableName.valueOf(nsp + TableName.NAMESPACE_DELIM + "table2")); + tableDescTwo.addFamily(fam1); HTableDescriptor tableDescThree = new HTableDescriptor(TableName.valueOf(nsp + TableName.NAMESPACE_DELIM + "table3")); + tableDescThree.addFamily(fam1); ADMIN.createTable(tableDescOne); boolean constraintViolated = false; try { @@ -252,10 +257,13 @@ public class TestNamespaceAuditor { assertNotNull("Namespace descriptor found null.", ADMIN.getNamespaceDescriptor(namespace)); NamespaceTableAndRegionInfo stateInfo = getNamespaceState(nspDesc.getName()); assertNotNull("Namespace state found null for " + namespace, stateInfo); + HColumnDescriptor fam1 = new HColumnDescriptor("fam1"); HTableDescriptor tableDescOne = new HTableDescriptor(TableName.valueOf(namespace + TableName.NAMESPACE_DELIM + "table1")); + tableDescOne.addFamily(fam1); HTableDescriptor tableDescTwo = new HTableDescriptor(TableName.valueOf(namespace + TableName.NAMESPACE_DELIM + "table2")); + tableDescTwo.addFamily(fam1); ADMIN.createTable(tableDescOne); ADMIN.createTable(tableDescTwo, Bytes.toBytes("AAA"), Bytes.toBytes("ZZZ"), 5); stateInfo = getNamespaceState(nspDesc.getName()); @@ -598,9 +606,13 @@ public class TestNamespaceAuditor { TableName tableOne = TableName.valueOf(nsp1 + TableName.NAMESPACE_DELIM + "table1"); TableName tableTwo = TableName.valueOf(nsp1 + TableName.NAMESPACE_DELIM + "table2"); TableName tableThree = TableName.valueOf(nsp1 + TableName.NAMESPACE_DELIM + "table3"); + HColumnDescriptor fam1 = new HColumnDescriptor("fam1"); HTableDescriptor tableDescOne = new HTableDescriptor(tableOne); + tableDescOne.addFamily(fam1); HTableDescriptor tableDescTwo = new HTableDescriptor(tableTwo); + tableDescTwo.addFamily(fam1); HTableDescriptor tableDescThree = new HTableDescriptor(tableThree); + tableDescThree.addFamily(fam1); ADMIN.createTable(tableDescOne, Bytes.toBytes("1"), Bytes.toBytes("1000"), 3); ADMIN.createTable(tableDescTwo, Bytes.toBytes("1"), Bytes.toBytes("1000"), 3); ADMIN.createTable(tableDescThree, Bytes.toBytes("1"), Bytes.toBytes("1000"), 4); @@ -690,10 +702,13 @@ public class TestNamespaceAuditor { ADMIN.createNamespace(nspDesc); assertNotNull("Namespace descriptor found null.", ADMIN.getNamespaceDescriptor(nsp)); assertEquals(ADMIN.listNamespaceDescriptors().length, 3); + HColumnDescriptor fam1 = new HColumnDescriptor("fam1"); HTableDescriptor tableDescOne = new HTableDescriptor(TableName.valueOf(nsp + TableName.NAMESPACE_DELIM + "table1")); + tableDescOne.addFamily(fam1); HTableDescriptor tableDescTwo = new HTableDescriptor(TableName.valueOf(nsp + TableName.NAMESPACE_DELIM + "table2")); + tableDescTwo.addFamily(fam1); ADMIN.createTable(tableDescOne); ADMIN.createTable(tableDescTwo, Bytes.toBytes("AAA"), Bytes.toBytes("ZZZ"), 4); } @@ -708,7 +723,9 @@ public class TestNamespaceAuditor { assertNotNull("Namespace descriptor found null.", ADMIN.getNamespaceDescriptor(nsp)); TableName tableName = TableName.valueOf(nsp + TableName.NAMESPACE_DELIM + "table1"); TableName cloneTableName = TableName.valueOf(nsp + TableName.NAMESPACE_DELIM + "table2"); + HColumnDescriptor fam1 = new HColumnDescriptor("fam1"); HTableDescriptor tableDescOne = new HTableDescriptor(tableName); + tableDescOne.addFamily(fam1); ADMIN.createTable(tableDescOne); String snapshot = "snapshot_testTableQuotaExceedWithCloneSnapshot"; ADMIN.snapshot(snapshot, tableName); @@ -726,7 +743,10 @@ public class TestNamespaceAuditor { assertNotNull("Namespace descriptor found null.", ADMIN.getNamespaceDescriptor(nsp)); TableName tableName = TableName.valueOf(nsp + TableName.NAMESPACE_DELIM + "table1"); TableName cloneTableName = TableName.valueOf(nsp + TableName.NAMESPACE_DELIM + "table2"); + + HColumnDescriptor fam1 = new HColumnDescriptor("fam1"); HTableDescriptor tableDescOne = new HTableDescriptor(tableName); + tableDescOne.addFamily(fam1); ADMIN.createTable(tableDescOne, Bytes.toBytes("AAA"), Bytes.toBytes("ZZZ"), 4); String snapshot = "snapshot_testCloneSnapshot"; @@ -761,6 +781,8 @@ public class TestNamespaceAuditor { assertNotNull("Namespace descriptor found null.", ADMIN.getNamespaceDescriptor(nsp)); TableName tableName1 = TableName.valueOf(nsp + TableName.NAMESPACE_DELIM + "table1"); HTableDescriptor tableDescOne = new HTableDescriptor(tableName1); + HColumnDescriptor fam1 = new HColumnDescriptor("fam1"); + tableDescOne.addFamily(fam1); ADMIN.createTable(tableDescOne, Bytes.toBytes("AAA"), Bytes.toBytes("ZZZ"), 4); NamespaceTableAndRegionInfo nstate = getNamespaceState(nsp); @@ -796,6 +818,9 @@ public class TestNamespaceAuditor { assertNotNull("Namespace descriptor found null.", ndesc); TableName tableName1 = TableName.valueOf(nsp + TableName.NAMESPACE_DELIM + "table1"); HTableDescriptor tableDescOne = new HTableDescriptor(tableName1); + HColumnDescriptor fam1 = new HColumnDescriptor("fam1"); + tableDescOne.addFamily(fam1); + ADMIN.createTable(tableDescOne, Bytes.toBytes("AAA"), Bytes.toBytes("ZZZ"), 4); NamespaceTableAndRegionInfo nstate = getNamespaceState(nsp); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionMover.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionMover.java index 0ecf1ebb3f8..a75ff2ba252 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionMover.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionMover.java @@ -35,6 +35,7 @@ import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.regionserver.HRegionServer; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.util.RegionMover.RegionMoverBuilder; +import org.apache.hadoop.hbase.HColumnDescriptor; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; @@ -70,6 +71,9 @@ public class TestRegionMover { TEST_UTIL.deleteTable(tableName); } HTableDescriptor tableDesc = new HTableDescriptor(tableName); + HColumnDescriptor fam1 = new HColumnDescriptor("fam1"); + tableDesc.addFamily(fam1); + try { admin.setBalancerRunning(false, true); String startKey = "a";