Index: src/main/java/org/apache/hadoop/hbase/backup/HFileArchiver.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/backup/HFileArchiver.java (revision 1423640) +++ src/main/java/org/apache/hadoop/hbase/backup/HFileArchiver.java (working copy) @@ -147,6 +147,38 @@ } /** + * Remove from the specified region the store files of the specified column family, + * either by archiving them or outright deletion + * @param fs the filesystem where the store files live + * @param conf {@link Configuration} to examine to determine the archive directory + * @param parent Parent region hosting the store files + * @param tableDir {@link Path} to where the table is being stored (for building the archive path) + * @param family the family hosting the store files + * @throws IOException if the files could not be correctly disposed. + */ + public static void archiveFamily(FileSystem fs, Configuration conf, + HRegionInfo parent, Path tableDir, byte[] family) throws IOException { + Path familyDir = new Path(tableDir, new Path(parent.getEncodedName(), Bytes.toString(family))); + FileStatus[] storeFiles = FSUtils.listStatus(fs, familyDir, null); + if (storeFiles == null) { + LOG.debug("No store files to dispose for region=" + parent.getRegionNameAsString() + + ", family=" + Bytes.toString(family)); + return; + } + + FileStatusConverter getAsFile = new FileStatusConverter(fs); + Collection toArchive = Lists.transform(Arrays.asList(storeFiles), getAsFile); + Path storeArchiveDir = HFileArchiveUtil.getStoreArchivePath(conf, parent, tableDir, family); + + // do the actual archive + if (!resolveAndArchive(fs, storeArchiveDir, toArchive)) { + throw new IOException("Failed to archive/delete all the files for region:" + + Bytes.toString(parent.getRegionName()) + ", family:" + Bytes.toString(family) + + " into " + storeArchiveDir + ". Something is probably awry on the filesystem."); + } + } + + /** * Remove the store files, either by archiving them or outright deletion * @param fs the filesystem where the store files live * @param parent Parent region hosting the store files @@ -196,7 +228,7 @@ if (!resolveAndArchive(fs, storeArchiveDir, storeFiles)) { throw new IOException("Failed to archive/delete all the files for region:" + Bytes.toString(parent.getRegionName()) + ", family:" + Bytes.toString(family) - + " into " + storeArchiveDir + "Something is probably arwy on the filesystem."); + + " into " + storeArchiveDir + ". Something is probably awry on the filesystem."); } } Index: src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java (revision 1423640) +++ src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java (working copy) @@ -454,6 +454,23 @@ // @see HRegion.checkRegioninfoOnFilesystem() } + public void deleteFamilyFromFS(HRegionInfo region, byte[] familyName) + throws IOException { + // archive family store files + Path tableDir = new Path(rootdir, region.getTableNameAsString()); + HFileArchiver.archiveFamily(fs, conf, region, tableDir, familyName); + + // delete the family folder + Path familyDir = new Path(tableDir, + new Path(region.getEncodedName(), Bytes.toString(familyName))); + if (fs.delete(familyDir, true) == false) { + throw new IOException("Could not delete family " + + Bytes.toString(familyName) + " from FileSystem for region " + + region.getRegionNameAsString() + "(" + region.getEncodedName() + + ")"); + } + } + public void stop() { if (splitLogManager != null) { this.splitLogManager.stop(); Index: src/main/java/org/apache/hadoop/hbase/master/handler/TableDeleteFamilyHandler.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/master/handler/TableDeleteFamilyHandler.java (revision 1423640) +++ src/main/java/org/apache/hadoop/hbase/master/handler/TableDeleteFamilyHandler.java (working copy) @@ -26,6 +26,7 @@ import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.Server; import org.apache.hadoop.hbase.master.MasterServices; +import org.apache.hadoop.hbase.master.MasterFileSystem; import org.apache.hadoop.hbase.util.Bytes; /** @@ -49,6 +50,12 @@ this.masterServices.getMasterFileSystem().deleteColumn(tableName, familyName); // Update in-memory descriptor cache this.masterServices.getTableDescriptors().add(htd); + // Remove the column family from the file system + MasterFileSystem mfs = this.masterServices.getMasterFileSystem(); + for (HRegionInfo hri : hris) { + // Delete the family directory in FS for all the regions one by one + mfs.deleteFamilyFromFS(hri, familyName); + } } @Override Index: src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java =================================================================== --- src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java (revision 1423640) +++ src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java (working copy) @@ -1015,6 +1015,36 @@ } /** + * Load table of multiple column families with rows from 'aaa' to 'zzz'. + * @param t Table + * @param f Array of Families to load + * @return Count of rows loaded. + * @throws IOException + */ + public int loadTable(final HTable t, final byte[][] f) throws IOException { + t.setAutoFlush(false); + byte[] k = new byte[3]; + int rowCount = 0; + for (byte b1 = 'a'; b1 <= 'z'; b1++) { + for (byte b2 = 'a'; b2 <= 'z'; b2++) { + for (byte b3 = 'a'; b3 <= 'z'; b3++) { + k[0] = b1; + k[1] = b2; + k[2] = b3; + Put put = new Put(k); + for (int i = 0; i < f.length; i++) { + put.add(f[i], null, k); + } + t.put(put); + rowCount++; + } + } + } + t.flushCommits(); + return rowCount; + } + + /** * Load region with rows from 'aaa' to 'zzz'. * @param r Region * @param f Family @@ -1148,6 +1178,9 @@ // and end key. Adding the custom regions below adds those blindly, // including the new start region from empty to "bbb". lg List rows = getMetaTableRows(htd.getName()); + String regionToDeleteInFS = table + .getRegionsInRange(Bytes.toBytes(""), Bytes.toBytes("")).get(0) + .getRegionInfo().getEncodedName(); List newRegions = new ArrayList(startKeys.length); // add custom ones int count = 0; @@ -1169,6 +1202,11 @@ Bytes.toStringBinary(row)); meta.delete(new Delete(row)); } + // remove the "old" region from FS + Path tableDir = new Path(getDefaultRootDirPath().toString() + + System.getProperty("file.separator") + htd.getNameAsString() + + System.getProperty("file.separator") + regionToDeleteInFS); + getDFSCluster().getFileSystem().delete(tableDir); // flush cache of regions HConnection conn = table.getConnection(); conn.clearRegionCache(); Index: src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java =================================================================== --- src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java (revision 1423640) +++ src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java (working copy) @@ -35,7 +35,7 @@ import org.apache.hadoop.fs.PathFilter; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HConstants; -import org.apache.hadoop.hbase.LargeTests; +import org.apache.hadoop.hbase.MediumTests; import org.apache.hadoop.hbase.client.HBaseAdmin; import org.apache.hadoop.hbase.regionserver.HRegion; import org.apache.hadoop.hbase.regionserver.HRegionServer; @@ -55,7 +55,7 @@ * Test that the {@link HFileArchiver} correctly removes all the parts of a region when cleaning up * a region */ -@Category(LargeTests.class) +@Category(MediumTests.class) public class TestHFileArchiving { private static final String STRING_TABLE_NAME = "test_table"; @@ -230,18 +230,70 @@ // then get the current store files Path regionDir = region.getRegionDir(); - List storeFiles = getAllFileNames(fs, regionDir); - // remove all the non-storefile named files for the region + List storeFiles = getRegionStoreFiles(fs, regionDir); + + // then delete the table so the hfiles get archived + UTIL.deleteTable(TABLE_NAME); + + // then get the files in the archive directory. + Path archiveDir = HFileArchiveUtil.getArchivePath(UTIL.getConfiguration()); + List archivedFiles = getAllFileNames(fs, archiveDir); + Collections.sort(storeFiles); + Collections.sort(archivedFiles); + + LOG.debug("Store files:"); for (int i = 0; i < storeFiles.size(); i++) { - String file = storeFiles.get(i); - if (file.contains(HRegion.REGIONINFO_FILE) || file.contains("hlog")) { - storeFiles.remove(i--); - } + LOG.debug(i + " - " + storeFiles.get(i)); } - storeFiles.remove(HRegion.REGIONINFO_FILE); + LOG.debug("Archive files:"); + for (int i = 0; i < archivedFiles.size(); i++) { + LOG.debug(i + " - " + archivedFiles.get(i)); + } + assertTrue("Archived files are missing some of the store files!", + archivedFiles.containsAll(storeFiles)); + } + + /** + * Test that the store files are archived when a column family is removed. + * @throws Exception + */ + @Test + public void testArchiveOnTableFamilyDelete() throws Exception { + List servingRegions = UTIL.getHBaseCluster().getRegions(TABLE_NAME); + // make sure we only have 1 region serving this table + assertEquals(1, servingRegions.size()); + HRegion region = servingRegions.get(0); + + // get the parent RS and monitor + HRegionServer hrs = UTIL.getRSForFirstRegionInTable(TABLE_NAME); + FileSystem fs = hrs.getFileSystem(); + + // put some data on the region + LOG.debug("-------Loading table"); + UTIL.loadRegion(region, TEST_FAM); + + // get the hfiles in the region + List regions = hrs.getOnlineRegions(TABLE_NAME); + assertEquals("More that 1 region for test table.", 1, regions.size()); + + region = regions.get(0); + // wait for all the compactions to complete + region.waitForFlushesAndCompactions(); + + // disable table to prevent new updates + UTIL.getHBaseAdmin().disableTable(TABLE_NAME); + LOG.debug("Disabled table"); + + // remove all the files from the archive to get a fair comparison + clearArchiveDirectory(); + + // then get the current store files + Path regionDir = region.getRegionDir(); + List storeFiles = getRegionStoreFiles(fs, regionDir); + // then delete the table so the hfiles get archived - UTIL.deleteTable(TABLE_NAME); + UTIL.getHBaseAdmin().deleteColumn(TABLE_NAME, TEST_FAM); // then get the files in the archive directory. Path archiveDir = HFileArchiveUtil.getArchivePath(UTIL.getConfiguration()); @@ -290,4 +342,18 @@ } return fileNames; } -} \ No newline at end of file + + private List getRegionStoreFiles(final FileSystem fs, final Path regionDir) + throws IOException { + List storeFiles = getAllFileNames(fs, regionDir); + // remove all the non-storefile named files for the region + for (int i = 0; i < storeFiles.size(); i++) { + String file = storeFiles.get(i); + if (file.contains(HRegion.REGIONINFO_FILE) || file.contains("hlog")) { + storeFiles.remove(i--); + } + } + storeFiles.remove(HRegion.REGIONINFO_FILE); + return storeFiles; + } +} Index: src/test/java/org/apache/hadoop/hbase/master/handler/TestTableDeleteFamilyHandler.java =================================================================== --- src/test/java/org/apache/hadoop/hbase/master/handler/TestTableDeleteFamilyHandler.java (revision 0) +++ src/test/java/org/apache/hadoop/hbase/master/handler/TestTableDeleteFamilyHandler.java (working copy) @@ -0,0 +1,159 @@ +/** + * Copyright The Apache Software Foundation + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.master.handler; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; + +import org.apache.hadoop.fs.FileStatus; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.HColumnDescriptor; +import org.apache.hadoop.hbase.HTableDescriptor; +import org.apache.hadoop.hbase.LargeTests; +import org.apache.hadoop.hbase.client.HBaseAdmin; +import org.apache.hadoop.hbase.client.HTable; +import org.apache.hadoop.hbase.util.Bytes; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +@Category(LargeTests.class) +public class TestTableDeleteFamilyHandler { + + private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); + private static final String TABLENAME = "column_family_handlers"; + private static final byte[][] FAMILIES = new byte[][] { Bytes.toBytes("cf1"), + Bytes.toBytes("cf2"), Bytes.toBytes("cf3") }; + + /** + * Start up a mini cluster and put a small table of empty regions into it. + * + * @throws Exception + */ + @BeforeClass + public static void beforeAllTests() throws Exception { + + TEST_UTIL.getConfiguration().setBoolean("dfs.support.append", true); + TEST_UTIL.startMiniCluster(2); + + // Create a table of three families. This will assign a region. + TEST_UTIL.createTable(Bytes.toBytes(TABLENAME), FAMILIES); + HTable t = new HTable(TEST_UTIL.getConfiguration(), TABLENAME); + + // Create multiple regions in all the three column families + TEST_UTIL.createMultiRegions(t, FAMILIES[0]); + + // Load the table with data for all families + TEST_UTIL.loadTable(t, FAMILIES); + + TEST_UTIL.flush(); + + t.close(); + } + + @AfterClass + public static void afterAllTests() throws Exception { + TEST_UTIL.deleteTable(Bytes.toBytes(TABLENAME)); + TEST_UTIL.shutdownMiniCluster(); + } + + @Before + public void setup() throws IOException, InterruptedException { + TEST_UTIL.ensureSomeRegionServersAvailable(2); + } + + @Test + public void deleteColumnFamilyWithMultipleRegions() throws Exception { + + HBaseAdmin admin = TEST_UTIL.getHBaseAdmin(); + HTableDescriptor beforehtd = admin.getTableDescriptor(Bytes + .toBytes(TABLENAME)); + + FileSystem fs = TEST_UTIL.getDFSCluster().getFileSystem(); + + // 1 - Check if table exists in descriptor + assertTrue(admin.isTableAvailable(TABLENAME)); + + // 2 - Check if all three families exist in descriptor + assertEquals(3, beforehtd.getColumnFamilies().length); + HColumnDescriptor[] families = beforehtd.getColumnFamilies(); + for (int i = 0; i < families.length; i++) { + + assertTrue(families[i].getNameAsString().equals("cf" + (i + 1))); + } + + // 3 - Check if table exists in FS + Path tableDir = new Path(TEST_UTIL.getDefaultRootDirPath().toString() + "/" + + TABLENAME); + assertTrue(fs.exists(tableDir)); + + // 4 - Check if all the 3 column families exist in FS + FileStatus[] fileStatus = fs.listStatus(tableDir); + for (int i = 0; i < fileStatus.length; i++) { + if (fileStatus[i].isDir() == true) { + FileStatus[] cf = fs.listStatus(fileStatus[i].getPath()); + int k = 1; + for (int j = 0; j < cf.length; j++) { + if (cf[j].isDir() == true + && cf[j].getPath().getName().startsWith(".") == false) { + assertTrue(cf[j].getPath().getName().equals("cf" + k)); + k++; + } + } + } + } + + // TEST - Disable and delete the column family + admin.disableTable(TABLENAME); + admin.deleteColumn(TABLENAME, "cf2"); + + // 5 - Check if only 2 column families exist in the descriptor + HTableDescriptor afterhtd = admin.getTableDescriptor(Bytes + .toBytes(TABLENAME)); + assertEquals(2, afterhtd.getColumnFamilies().length); + HColumnDescriptor[] newFamilies = afterhtd.getColumnFamilies(); + assertTrue(newFamilies[0].getNameAsString().equals("cf1")); + assertTrue(newFamilies[1].getNameAsString().equals("cf3")); + + // 6 - Check if the second column family is gone from the FS + fileStatus = fs.listStatus(tableDir); + for (int i = 0; i < fileStatus.length; i++) { + if (fileStatus[i].isDir() == true) { + FileStatus[] cf = fs.listStatus(fileStatus[i].getPath()); + for (int j = 0; j < cf.length; j++) { + if (cf[j].isDir() == true) { + assertFalse(cf[j].getPath().getName().equals("cf2")); + } + } + } + } + } + + @org.junit.Rule + public org.apache.hadoop.hbase.ResourceCheckerJUnitRule cu = + new org.apache.hadoop.hbase.ResourceCheckerJUnitRule(); +}