diff --git a/CHANGES.txt b/CHANGES.txt index e9ef89ba9ba..8ebd65a660f 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -464,6 +464,7 @@ Release 0.92.0 - Unreleased HBASE-4754 FSTableDescriptors.getTableInfoPath() should handle FileNotFoundException HBASE-4740 [bulk load] the HBASE-4552 API can't tell if errors on region server are recoverable (Jonathan Hsieh) + HBASE-4741 Online schema change doesn't return errors TESTS HBASE-4450 test for number of blocks read: to serve as baseline for expected diff --git a/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java b/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java index 0480d1eccf0..c300a32d07a 100644 --- a/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java +++ b/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java @@ -129,7 +129,7 @@ public class HBaseAdmin implements Abortable, Closeable { * to cleanup the returned catalog tracker. * @throws ZooKeeperConnectionException * @throws IOException - * @see #cleanupCatalogTracker(CatalogTracker); + * @see #cleanupCatalogTracker(CatalogTracker) */ private synchronized CatalogTracker getCatalogTracker() throws ZooKeeperConnectionException, IOException { diff --git a/src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java b/src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java index 000f99c4fa3..9e0535e576d 100644 --- a/src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java +++ b/src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java @@ -24,14 +24,9 @@ import java.util.List; import org.apache.hadoop.hbase.ClusterStatus; import org.apache.hadoop.hbase.HColumnDescriptor; -import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.UnknownRegionException; import org.apache.hadoop.hbase.util.Pair; -import org.apache.hadoop.hbase.coprocessor.ObserverContext; -import org.apache.hadoop.hbase.ipc.VersionedProtocol; - - /** * Clients interact with the HMasterInterface to gain access to meta-level diff --git a/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java b/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java index caa1fe07939..d5b598fad81 100644 --- a/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java +++ b/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java @@ -40,6 +40,8 @@ public class DeleteTableHandler extends TableEventHandler { final MasterServices masterServices) throws IOException { super(EventType.C_M_DELETE_TABLE, tableName, server, masterServices); + // The next call fails if no such table. + getTableDescriptor(); } @Override diff --git a/src/main/java/org/apache/hadoop/hbase/master/handler/ModifyTableHandler.java b/src/main/java/org/apache/hadoop/hbase/master/handler/ModifyTableHandler.java index 3d72463032a..f08d80dd80c 100644 --- a/src/main/java/org/apache/hadoop/hbase/master/handler/ModifyTableHandler.java +++ b/src/main/java/org/apache/hadoop/hbase/master/handler/ModifyTableHandler.java @@ -26,20 +26,19 @@ import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.Server; import org.apache.hadoop.hbase.master.MasterServices; -import org.apache.hadoop.hbase.util.Bytes; public class ModifyTableHandler extends TableEventHandler { private final HTableDescriptor htd; public ModifyTableHandler(final byte [] tableName, final HTableDescriptor htd, final Server server, - final MasterServices masterServices) throws IOException { + final MasterServices masterServices) + throws IOException { super(EventType.C_M_MODIFY_TABLE, tableName, server, masterServices); + // Check table exists. + getTableDescriptor(); + // This is the new schema we are going to write out as this modification. this.htd = htd; - if (!Bytes.equals(tableName, htd.getName())) { - throw new IOException("TableDescriptor name & tableName must match: " - + htd.getNameAsString() + " vs " + Bytes.toString(tableName)); - } } @Override @@ -55,6 +54,7 @@ public class ModifyTableHandler extends TableEventHandler { if(server != null && server.getServerName() != null) { name = server.getServerName().toString(); } - return getClass().getSimpleName() + "-" + name + "-" + getSeqid() + "-" + tableNameStr; + return getClass().getSimpleName() + "-" + name + "-" + getSeqid() + "-" + + tableNameStr; } -} +} \ No newline at end of file diff --git a/src/main/java/org/apache/hadoop/hbase/master/handler/TableAddFamilyHandler.java b/src/main/java/org/apache/hadoop/hbase/master/handler/TableAddFamilyHandler.java index 18cff0b3208..3fb50b2efeb 100644 --- a/src/main/java/org/apache/hadoop/hbase/master/handler/TableAddFamilyHandler.java +++ b/src/main/java/org/apache/hadoop/hbase/master/handler/TableAddFamilyHandler.java @@ -28,7 +28,6 @@ import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.InvalidFamilyOperationException; import org.apache.hadoop.hbase.Server; import org.apache.hadoop.hbase.master.MasterServices; -import org.apache.hadoop.hbase.util.Bytes; /** * Handles adding a new family to an existing table. @@ -40,27 +39,19 @@ public class TableAddFamilyHandler extends TableEventHandler { public TableAddFamilyHandler(byte[] tableName, HColumnDescriptor familyDesc, Server server, final MasterServices masterServices) throws IOException { super(EventType.C_M_ADD_FAMILY, tableName, server, masterServices); + HTableDescriptor htd = getTableDescriptor(); + if (htd.hasFamily(familyDesc.getName())) { + throw new InvalidFamilyOperationException("Family '" + + familyDesc.getNameAsString() + "' already exists so cannot be added"); + } this.familyDesc = familyDesc; } @Override protected void handleTableOperation(List hris) throws IOException { - HTableDescriptor htd = this.masterServices.getTableDescriptors(). - get(Bytes.toString(tableName)); - byte [] familyName = familyDesc.getName(); - if (htd == null) { - throw new IOException("Add Family operation could not be completed as " + - "HTableDescritor is missing for table = " - + Bytes.toString(tableName)); - } - if(htd.hasFamily(familyName)) { - throw new InvalidFamilyOperationException( - "Family '" + Bytes.toString(familyName) + "' already exists so " + - "cannot be added"); - } // Update table descriptor in HDFS - htd = this.masterServices.getMasterFileSystem() + HTableDescriptor htd = this.masterServices.getMasterFileSystem() .addColumn(tableName, familyDesc); // Update in-memory descriptor cache this.masterServices.getTableDescriptors().add(htd); diff --git a/src/main/java/org/apache/hadoop/hbase/master/handler/TableDeleteFamilyHandler.java b/src/main/java/org/apache/hadoop/hbase/master/handler/TableDeleteFamilyHandler.java index 2f65ff93f4c..89889316744 100644 --- a/src/main/java/org/apache/hadoop/hbase/master/handler/TableDeleteFamilyHandler.java +++ b/src/main/java/org/apache/hadoop/hbase/master/handler/TableDeleteFamilyHandler.java @@ -24,9 +24,7 @@ import java.util.List; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.HTableDescriptor; -import org.apache.hadoop.hbase.InvalidFamilyOperationException; import org.apache.hadoop.hbase.Server; -import org.apache.hadoop.hbase.master.AssignmentManager; import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.util.Bytes; @@ -40,26 +38,15 @@ public class TableDeleteFamilyHandler extends TableEventHandler { public TableDeleteFamilyHandler(byte[] tableName, byte [] familyName, Server server, final MasterServices masterServices) throws IOException { super(EventType.C_M_ADD_FAMILY, tableName, server, masterServices); - this.familyName = familyName; + HTableDescriptor htd = getTableDescriptor(); + this.familyName = hasColumnFamily(htd, familyName); } @Override protected void handleTableOperation(List hris) throws IOException { - AssignmentManager am = this.masterServices.getAssignmentManager(); - HTableDescriptor htd = this.masterServices.getTableDescriptors().get(Bytes.toString(tableName)); - if (htd == null) { - throw new IOException("Add Family operation could not be completed as " + - "HTableDescritor is missing for table = " - + Bytes.toString(tableName)); - } - if(!htd.hasFamily(familyName)) { - throw new InvalidFamilyOperationException( - "Family '" + Bytes.toString(familyName) + "' does not exist so " + - "cannot be deleted"); - } // Update table descriptor in HDFS - htd = this.masterServices.getMasterFileSystem() - .deleteColumn(tableName, familyName); + HTableDescriptor htd = + this.masterServices.getMasterFileSystem().deleteColumn(tableName, familyName); // Update in-memory descriptor cache this.masterServices.getTableDescriptors().add(htd); } diff --git a/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java b/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java index 59fc758a1bb..a88eb887adf 100644 --- a/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java +++ b/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java @@ -19,6 +19,7 @@ */ package org.apache.hadoop.hbase.master.handler; +import java.io.FileNotFoundException; import java.io.IOException; import java.util.ArrayList; import java.util.LinkedList; @@ -29,8 +30,11 @@ import java.util.TreeMap; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.hbase.HRegionInfo; +import org.apache.hadoop.hbase.HTableDescriptor; +import org.apache.hadoop.hbase.InvalidFamilyOperationException; import org.apache.hadoop.hbase.Server; import org.apache.hadoop.hbase.ServerName; +import org.apache.hadoop.hbase.TableExistsException; import org.apache.hadoop.hbase.TableNotDisabledException; import org.apache.hadoop.hbase.catalog.MetaReader; import org.apache.hadoop.hbase.client.HTable; @@ -148,6 +152,33 @@ public abstract class TableEventHandler extends EventHandler { } return done; } + + /** + * @return Table descriptor for this table + * @throws TableExistsException + * @throws FileNotFoundException + * @throws IOException + */ + HTableDescriptor getTableDescriptor() + throws TableExistsException, FileNotFoundException, IOException { + final String name = Bytes.toString(tableName); + HTableDescriptor htd = + this.masterServices.getTableDescriptors().get(name); + if (htd == null) { + throw new IOException("HTableDescriptor missing for " + name); + } + return htd; + } + + byte [] hasColumnFamily(final HTableDescriptor htd, final byte [] cf) + throws InvalidFamilyOperationException { + if (!htd.hasFamily(cf)) { + throw new InvalidFamilyOperationException("Column family '" + + Bytes.toString(cf) + "' does not exist"); + } + return cf; + } + protected abstract void handleTableOperation(List regions) throws IOException, KeeperException; } diff --git a/src/main/java/org/apache/hadoop/hbase/master/handler/TableModifyFamilyHandler.java b/src/main/java/org/apache/hadoop/hbase/master/handler/TableModifyFamilyHandler.java index 708ee73ecf6..34560ec674c 100644 --- a/src/main/java/org/apache/hadoop/hbase/master/handler/TableModifyFamilyHandler.java +++ b/src/main/java/org/apache/hadoop/hbase/master/handler/TableModifyFamilyHandler.java @@ -27,7 +27,6 @@ import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.InvalidFamilyOperationException; import org.apache.hadoop.hbase.Server; -import org.apache.hadoop.hbase.master.AssignmentManager; import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.util.Bytes; @@ -35,35 +34,26 @@ import org.apache.hadoop.hbase.util.Bytes; * Handles adding a new family to an existing table. */ public class TableModifyFamilyHandler extends TableEventHandler { - private final HColumnDescriptor familyDesc; public TableModifyFamilyHandler(byte[] tableName, HColumnDescriptor familyDesc, Server server, final MasterServices masterServices) throws IOException { super(EventType.C_M_MODIFY_FAMILY, tableName, server, masterServices); + HTableDescriptor htd = getTableDescriptor(); + hasColumnFamily(htd, familyDesc.getName()); this.familyDesc = familyDesc; } @Override protected void handleTableOperation(List regions) throws IOException { - AssignmentManager am = this.masterServices.getAssignmentManager(); - HTableDescriptor htd = this.masterServices.getTableDescriptors().get(Bytes.toString(tableName)); - byte [] familyName = familyDesc.getName(); - if (htd == null) { - throw new IOException("Modify Family operation could not be completed as " + - "HTableDescritor is missing for table = " - + Bytes.toString(tableName)); - } - if(!htd.hasFamily(familyName)) { - throw new InvalidFamilyOperationException("Family '" + - Bytes.toString(familyName) + "' doesn't exists so cannot be modified"); - } // Update table descriptor in HDFS - htd = this.masterServices.getMasterFileSystem().modifyColumn(tableName, familyDesc); + HTableDescriptor htd = + this.masterServices.getMasterFileSystem().modifyColumn(tableName, familyDesc); // Update in-memory descriptor cache this.masterServices.getTableDescriptors().add(htd); } + @Override public String toString() { String name = "UnknownServerName"; diff --git a/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java b/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java index 82fca2f615b..25c0eb6e1dc 100644 --- a/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java +++ b/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java @@ -47,6 +47,7 @@ import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.regionserver.HRegion; import org.apache.hadoop.hbase.regionserver.HRegionServer; import org.apache.hadoop.hbase.regionserver.wal.TestHLogUtils; +import org.apache.hadoop.hbase.InvalidFamilyOperationException; import org.apache.hadoop.hbase.util.Bytes; import org.junit.AfterClass; import org.junit.Before; @@ -86,6 +87,96 @@ public class TestAdmin { this.admin = new HBaseAdmin(TEST_UTIL.getConfiguration()); } + @Test + public void testDeleteEditUnknownColumnFamilyAndOrTable() throws IOException { + // Test we get exception if we try to + final String nonexistent = "nonexistent"; + HColumnDescriptor nonexistentHcd = new HColumnDescriptor(nonexistent); + Exception exception = null; + try { + this.admin.addColumn(nonexistent, nonexistentHcd); + } catch (IOException e) { + exception = e; + } + assertTrue(exception instanceof TableNotFoundException); + + exception = null; + try { + this.admin.deleteTable(nonexistent); + } catch (IOException e) { + exception = e; + } + assertTrue(exception instanceof TableNotFoundException); + + exception = null; + try { + this.admin.deleteColumn(nonexistent, nonexistent); + } catch (IOException e) { + exception = e; + } + assertTrue(exception instanceof TableNotFoundException); + + exception = null; + try { + this.admin.disableTable(nonexistent); + } catch (IOException e) { + exception = e; + } + assertTrue(exception instanceof TableNotFoundException); + + exception = null; + try { + this.admin.enableTable(nonexistent); + } catch (IOException e) { + exception = e; + } + assertTrue(exception instanceof TableNotFoundException); + + exception = null; + try { + this.admin.modifyColumn(nonexistent, nonexistentHcd); + } catch (IOException e) { + exception = e; + } + assertTrue(exception instanceof TableNotFoundException); + + exception = null; + try { + HTableDescriptor htd = new HTableDescriptor(nonexistent); + this.admin.modifyTable(htd.getName(), htd); + } catch (IOException e) { + exception = e; + } + assertTrue(exception instanceof TableNotFoundException); + + // Now make it so at least the table exists and then do tests against a + // nonexistent column family -- see if we get right exceptions. + final String tableName = "t"; + HTableDescriptor htd = new HTableDescriptor(tableName); + htd.addFamily(new HColumnDescriptor("cf")); + this.admin.createTable(htd); + try { + exception = null; + try { + this.admin.deleteColumn(htd.getName(), nonexistentHcd.getName()); + } catch (IOException e) { + exception = e; + } + assertTrue(exception instanceof InvalidFamilyOperationException); + + exception = null; + try { + this.admin.modifyColumn(htd.getName(), nonexistentHcd); + } catch (IOException e) { + exception = e; + } + assertTrue(exception instanceof InvalidFamilyOperationException); + } finally { + this.admin.disableTable(tableName); + this.admin.deleteTable(tableName); + } + } + @Test public void testDisableAndEnableTable() throws IOException { final byte [] row = Bytes.toBytes("row"); @@ -218,7 +309,6 @@ public class TestAdmin { public void testOnlineChangeTableSchema() throws IOException, InterruptedException { final byte [] tableName = Bytes.toBytes("changeTableSchemaOnline"); HTableDescriptor [] tables = admin.listTables(); - MasterServices masterServices = TEST_UTIL.getMiniHBaseCluster().getMaster(); int numTables = tables.length; TEST_UTIL.createTable(tableName, HConstants.CATALOG_FAMILY); tables = this.admin.listTables(); @@ -243,7 +333,6 @@ public class TestAdmin { } assertFalse(expectedException); HTableDescriptor modifiedHtd = this.admin.getTableDescriptor(tableName); - // Assert returned modifiedhcd is same as the copy. assertFalse(htd.equals(modifiedHtd)); assertTrue(copy.equals(modifiedHtd)); assertEquals(newFlushSize, modifiedHtd.getMemStoreFlushSize()); @@ -784,7 +873,7 @@ public class TestAdmin { @Test (expected=IllegalArgumentException.class) public void testInvalidHColumnDescriptor() throws IOException { - HColumnDescriptor hcd = new HColumnDescriptor("/cfamily/name"); + new HColumnDescriptor("/cfamily/name"); } @Test @@ -801,11 +890,10 @@ public class TestAdmin { this.admin.enableTable(tableName); try { this.admin.deleteColumn(tableName, Bytes.toBytes("col2")); - } catch(TableNotDisabledException e) { - // Expected + } catch (TableNotDisabledException e) { + LOG.info(e); } this.admin.disableTable(tableName); - this.admin.deleteColumn(tableName, Bytes.toBytes("col2")); this.admin.deleteTable(tableName); }