HBASE-6564 HDFS space is not reclaimed when a column family is deleted (J Mohamed)

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1378648 13f79535-47bb-0310-9956-ffa450edef68

hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java

@@ -461,6 +461,19 @@ public class MasterFileSystem {
     //      @see HRegion.checkRegioninfoOnFilesystem()
   }
 
+  public void deleteFamilyFromFS(HRegionInfo region, byte[] familyName)
+      throws IOException {
+    Path delDir = new Path(rootdir,
+        new Path(region.getTableNameAsString(), new Path(
+            region.getEncodedName(), new Path(Bytes.toString(familyName)))));
+    if (fs.delete(delDir, 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();

hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/TableDeleteFamilyHandler.java

@@ -28,6 +28,7 @@ import org.apache.hadoop.hbase.HTableDescriptor;
 import org.apache.hadoop.hbase.Server;
 import org.apache.hadoop.hbase.master.HMaster;
 import org.apache.hadoop.hbase.master.MasterCoprocessorHost;
+import org.apache.hadoop.hbase.master.MasterFileSystem;
 import org.apache.hadoop.hbase.master.MasterServices;
 import org.apache.hadoop.hbase.util.Bytes;
 
@@ -58,6 +59,12 @@ public class TableDeleteFamilyHandler extends TableEventHandler {
       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);
+    }
     if (cpHost != null) {
       cpHost.postDeleteColumnHandler(this.tableName, this.familyName);
     }

hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java

@@ -72,9 +72,9 @@ import org.apache.hadoop.hbase.mapreduce.MapreduceTestingShim;
 import org.apache.hadoop.hbase.master.HMaster;
 import org.apache.hadoop.hbase.regionserver.HRegion;
 import org.apache.hadoop.hbase.regionserver.HRegionServer;
+import org.apache.hadoop.hbase.regionserver.HStore;
 import org.apache.hadoop.hbase.regionserver.InternalScanner;
 import org.apache.hadoop.hbase.regionserver.MultiVersionConsistencyControl;
-import org.apache.hadoop.hbase.regionserver.HStore;
 import org.apache.hadoop.hbase.regionserver.StoreFile;
 import org.apache.hadoop.hbase.security.User;
 import org.apache.hadoop.hbase.util.Bytes;
@@ -1047,6 +1047,39 @@ public class HBaseTestingUtility {
     t.flushCommits();
     return rowCount;
   }
+  
+  
+  /**
+   * 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
@@ -1181,6 +1214,9 @@ public class HBaseTestingUtility {
     // and end key. Adding the custom regions below adds those blindly,
     // including the new start region from empty to "bbb". lg
     List<byte[]> rows = getMetaTableRows(htd.getName());
+    String regionToDeleteInFS = table
+        .getRegionsInRange(Bytes.toBytes(""), Bytes.toBytes("")).get(0)
+        .getRegionInfo().getEncodedName();
     List<HRegionInfo> newRegions = new ArrayList<HRegionInfo>(startKeys.length);
     // add custom ones
     int count = 0;
@@ -1198,6 +1234,11 @@ public class HBaseTestingUtility {
         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();

hbase-server/src/test/java/org/apache/hadoop/hbase/master/handler/TestTableDeleteFamilyHandler.java

@@ -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();
+}