From 13483015c99757620bbbb1f7e527bd7792328103 Mon Sep 17 00:00:00 2001
From: mbertozzi <mbertozzi@unknown>
Date: Fri, 17 May 2013 15:54:31 +0000
Subject: [PATCH] HBASE-7726 Family Dir is not removed using modifyTable()

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1483883 13f79535-47bb-0310-9956-ffa450edef68
---
 .../hadoop/hbase/client/HBaseAdmin.java       |  5 +++
 .../master/handler/ModifyTableHandler.java    | 34 +++++++++++++++++--
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
index 0972aeb98d5..6d0b5fb894d 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
@@ -1813,6 +1813,11 @@ public class HBaseAdmin implements Abortable, Closeable {
    */
   public void modifyTable(final byte [] tableName, final HTableDescriptor htd)
   throws IOException {
+    if (!Bytes.equals(tableName, htd.getName())) {
+      throw new IllegalArgumentException("the specified table name '" + Bytes.toString(tableName) +
+        "' doesn't match with the HTD one: " + htd.getNameAsString());
+    }
+
     execute(new MasterAdminCallable<Void>() {
       @Override
       public Void call() throws ServiceException {
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/ModifyTableHandler.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/ModifyTableHandler.java
index 84cad249532..e8a8df9c985 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/ModifyTableHandler.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/ModifyTableHandler.java
@@ -20,7 +20,10 @@ package org.apache.hadoop.hbase.master.handler;
 
 import java.io.IOException;
 import java.util.List;
+import java.util.Set;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.hbase.HRegionInfo;
 import org.apache.hadoop.hbase.HTableDescriptor;
@@ -28,10 +31,14 @@ import org.apache.hadoop.hbase.Server;
 import org.apache.hadoop.hbase.executor.EventType;
 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;
 
 @InterfaceAudience.Private
 public class ModifyTableHandler extends TableEventHandler {
+  private static final Log LOG = LogFactory.getLog(ModifyTableHandler.class);
+
   private final HTableDescriptor htd;
 
   public ModifyTableHandler(final byte [] tableName,
@@ -52,18 +59,41 @@ public class ModifyTableHandler extends TableEventHandler {
   @Override
   protected void handleTableOperation(List<HRegionInfo> hris)
   throws IOException {
-    MasterCoprocessorHost cpHost = ((HMaster) this.server)
-        .getCoprocessorHost();
+    MasterCoprocessorHost cpHost = ((HMaster) this.server).getCoprocessorHost();
     if (cpHost != null) {
       cpHost.preModifyTableHandler(this.tableName, this.htd);
     }
     // Update descriptor
+    HTableDescriptor oldHtd = getTableDescriptor();
     this.masterServices.getTableDescriptors().add(this.htd);
+    deleteFamilyFromFS(hris, oldHtd.getFamiliesKeys());
     if (cpHost != null) {
       cpHost.postModifyTableHandler(this.tableName, this.htd);
     }
   }
 
+  /**
+   * Removes from hdfs the families that are not longer present in the new table descriptor.
+   */
+  private void deleteFamilyFromFS(final List<HRegionInfo> hris, final Set<byte[]> oldFamilies) {
+    try {
+      Set<byte[]> newFamilies = this.htd.getFamiliesKeys();
+      MasterFileSystem mfs = this.masterServices.getMasterFileSystem();
+      for (byte[] familyName: oldFamilies) {
+        if (!newFamilies.contains(familyName)) {
+          LOG.debug("Removing family=" + Bytes.toString(familyName) +
+                    " from table=" + this.tableName);
+          for (HRegionInfo hri: hris) {
+            // Delete the family directory in FS for all the regions one by one
+            mfs.deleteFamilyFromFS(hri, familyName);
+          }
+        }
+      }
+    } catch (IOException e) {
+      LOG.warn("Unable to remove on-disk directories for the removed families", e);
+    }
+  }
+
   @Override
   public String toString() {
     String name = "UnknownServerName";