HBASE-26454 CreateTableProcedure still relies on temp dir and renames… (#3845)

Signed-off-by: Duo Zhang <zhangduo@apache.org>

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

@@ -22,7 +22,6 @@ package org.apache.hadoop.hbase.master.procedure;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
-import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hbase.DoNotRetryIOException;
 import org.apache.hadoop.hbase.HBaseIOException;
@@ -316,41 +315,22 @@ public class CreateTableProcedure
       final TableDescriptor tableDescriptor, List<RegionInfo> newRegions,
       final CreateHdfsRegions hdfsRegionHandler) throws IOException {
     final MasterFileSystem mfs = env.getMasterServices().getMasterFileSystem();
-    final Path tempdir = mfs.getTempDir();
 
     // 1. Create Table Descriptor
     // using a copy of descriptor, table will be created enabling first
-    final Path tempTableDir = CommonFSUtils.getTableDir(tempdir, tableDescriptor.getTableName());
+    final Path tableDir = CommonFSUtils.getTableDir(mfs.getRootDir(),
+      tableDescriptor.getTableName());
     ((FSTableDescriptors)(env.getMasterServices().getTableDescriptors()))
-        .createTableDescriptorForTableDirectory(tempTableDir, tableDescriptor, false);
+        .createTableDescriptorForTableDirectory(
+          tableDir, tableDescriptor, false);
 
     // 2. Create Regions
-    newRegions = hdfsRegionHandler.createHdfsRegions(env, tempdir,
+    newRegions = hdfsRegionHandler.createHdfsRegions(env, mfs.getRootDir(),
             tableDescriptor.getTableName(), newRegions);
 
-    // 3. Move Table temp directory to the hbase root location
-    moveTempDirectoryToHBaseRoot(env, tableDescriptor, tempTableDir);
-
     return newRegions;
   }
 
-  protected static void moveTempDirectoryToHBaseRoot(
-    final MasterProcedureEnv env,
-    final TableDescriptor tableDescriptor,
-    final Path tempTableDir) throws IOException {
-    final MasterFileSystem mfs = env.getMasterServices().getMasterFileSystem();
-    final Path tableDir =
-      CommonFSUtils.getTableDir(mfs.getRootDir(), tableDescriptor.getTableName());
-    FileSystem fs = mfs.getFileSystem();
-    if (!fs.delete(tableDir, true) && fs.exists(tableDir)) {
-      throw new IOException("Couldn't delete " + tableDir);
-    }
-    if (!fs.rename(tempTableDir, tableDir)) {
-      throw new IOException("Unable to move table from temp=" + tempTableDir +
-        " to hbase root=" + tableDir);
-    }
-  }
-
   protected static List<RegionInfo> addTableToMeta(final MasterProcedureEnv env,
     final TableDescriptor tableDescriptor, final List<RegionInfo> regions) throws IOException {
     assert (regions != null && regions.size() > 0) : "expected at least 1 region, got " + regions;

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

@@ -20,10 +20,7 @@ package org.apache.hadoop.hbase.master.procedure;
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.List;
-import java.util.stream.Collectors;
-import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hbase.MetaTableAccessor;
@@ -52,11 +49,12 @@ import org.apache.yetus.audience.InterfaceAudience;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.hbase.thirdparty.org.apache.commons.collections4.CollectionUtils;
+
 import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos.DeleteTableState;
-import org.apache.hbase.thirdparty.org.apache.commons.collections4.CollectionUtils;
 
 @InterfaceAudience.Private
 public class DeleteTableProcedure
@@ -278,59 +276,26 @@ public class DeleteTableProcedure
       final boolean archive) throws IOException {
     final MasterFileSystem mfs = env.getMasterServices().getMasterFileSystem();
     final FileSystem fs = mfs.getFileSystem();
-    final Path tempdir = mfs.getTempDir();
 
     final Path tableDir = CommonFSUtils.getTableDir(mfs.getRootDir(), tableName);
-    final Path tempTableDir = CommonFSUtils.getTableDir(tempdir, tableName);
 
     if (fs.exists(tableDir)) {
-      // Ensure temp exists
-      if (!fs.exists(tempdir) && !fs.mkdirs(tempdir)) {
-        throw new IOException("HBase temp directory '" + tempdir + "' creation failure.");
-      }
-
-      // Ensure parent exists
-      if (!fs.exists(tempTableDir.getParent()) && !fs.mkdirs(tempTableDir.getParent())) {
-        throw new IOException("HBase temp directory '" + tempdir + "' creation failure.");
-      }
-
-      if (fs.exists(tempTableDir)) {
-        // TODO
-        // what's in this dir? something old? probably something manual from the user...
-        // let's get rid of this stuff...
-        FileStatus[] files = fs.listStatus(tempTableDir);
-        if (files != null && files.length > 0) {
-          List<Path> regionDirList = Arrays.stream(files)
-            .filter(FileStatus::isDirectory)
-            .map(FileStatus::getPath)
-            .collect(Collectors.toList());
-          HFileArchiver.archiveRegions(env.getMasterConfiguration(), fs, mfs.getRootDir(),
-            tempTableDir, regionDirList);
-        }
-        fs.delete(tempTableDir, true);
-      }
-
-      // Move the table in /hbase/.tmp
-      if (!fs.rename(tableDir, tempTableDir)) {
-        throw new IOException("Unable to move '" + tableDir + "' to temp '" + tempTableDir + "'");
-      }
-    }
-
       // Archive regions from FS (temp directory)
       if (archive) {
         List<Path> regionDirList = new ArrayList<>();
         for (RegionInfo region : regions) {
           if (RegionReplicaUtil.isDefaultReplica(region)) {
-          regionDirList.add(FSUtils.getRegionDirFromTableDir(tempTableDir, region));
+            regionDirList.add(FSUtils.getRegionDirFromTableDir(tableDir, region));
             List<RegionInfo> mergeRegions = MetaTableAccessor
               .getMergeRegions(env.getMasterServices().getConnection(), region.getRegionName());
             if (!CollectionUtils.isEmpty(mergeRegions)) {
-            mergeRegions.stream()
+              mergeRegions.stream().forEach(
-                .forEach(r -> regionDirList.add(FSUtils.getRegionDirFromTableDir(tempTableDir, r)));
+                r -> regionDirList.add(FSUtils.getRegionDirFromTableDir(tableDir, r)));
             }
           }
         }
-      HFileArchiver.archiveRegions(env.getMasterConfiguration(), fs, mfs.getRootDir(), tempTableDir,
+        HFileArchiver
+          .archiveRegions(env.getMasterConfiguration(), fs, mfs.getRootDir(), tableDir,
             regionDirList);
         if (!regionDirList.isEmpty()) {
           LOG.debug("Archived {} regions", tableName);
@@ -340,15 +305,14 @@ public class DeleteTableProcedure
       // Archive mob data
       Path mobTableDir =
         CommonFSUtils.getTableDir(new Path(mfs.getRootDir(), MobConstants.MOB_DIR_NAME), tableName);
-    Path regionDir =
+      Path regionDir = new Path(mobTableDir, MobUtils.getMobRegionInfo(tableName).getEncodedName());
-            new Path(mobTableDir, MobUtils.getMobRegionInfo(tableName).getEncodedName());
       if (fs.exists(regionDir)) {
         HFileArchiver.archiveRegion(fs, mfs.getRootDir(), mobTableDir, regionDir);
       }
 
-    // Delete table directory from FS (temp directory)
+      // Delete table directory from FS
-    if (!fs.delete(tempTableDir, true) && fs.exists(tempTableDir)) {
+      if (!fs.delete(tableDir, true) && fs.exists(tableDir)) {
-      throw new IOException("Couldn't delete " + tempTableDir);
+        throw new IOException("Couldn't delete " + tableDir);
       }
 
       // Delete the table directory where the mob files are saved
@@ -365,6 +329,7 @@ public class DeleteTableProcedure
         throw new IOException("Couldn't delete table dir on wal filesystem" + tableWALDir);
       }
     }
+  }
 
   /**
    * There may be items for this table still up in hbase:meta in the case where the info:regioninfo

hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclHelper.java

@@ -478,8 +478,8 @@ public class SnapshotScannerHDFSAclHelper implements Closeable {
    */
   List<Path> getTableRootPaths(TableName tableName, boolean includeSnapshotPath)
       throws IOException {
-    List<Path> paths = Lists.newArrayList(pathHelper.getTmpTableDir(tableName),
+    List<Path> paths = Lists.newArrayList(pathHelper.getDataTableDir(tableName),
-      pathHelper.getDataTableDir(tableName), pathHelper.getMobTableDir(tableName),
+      pathHelper.getMobTableDir(tableName),
       pathHelper.getArchiveTableDir(tableName));
     if (includeSnapshotPath) {
       paths.addAll(getTableSnapshotPaths(tableName));

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

@@ -18,8 +18,7 @@
 package org.apache.hadoop.hbase.master;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.fail;
 
 import java.util.List;
 import org.apache.hadoop.fs.FileSystem;
@@ -33,7 +32,6 @@ import org.apache.hadoop.hbase.testclassification.MasterTests;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.CommonFSUtils;
-import org.apache.hadoop.hbase.util.HFileArchiveTestingUtil;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
@@ -85,7 +83,7 @@ public class TestMasterFileSystem {
   }
 
   @Test
-  public void testCheckTempDir() throws Exception {
+  public void testCheckNoTempDir() throws Exception {
     final MasterFileSystem masterFileSystem =
       UTIL.getMiniHBaseCluster().getMaster().getMasterFileSystem();
 
@@ -110,28 +108,13 @@ public class TestMasterFileSystem {
     // disable the table so that we can manipulate the files
     UTIL.getAdmin().disableTable(tableName);
 
-    final Path tableDir = CommonFSUtils.getTableDir(masterFileSystem.getRootDir(), tableName);
     final Path tempDir = masterFileSystem.getTempDir();
-    final Path tempTableDir = CommonFSUtils.getTableDir(tempDir, tableName);
+    final Path tempNsDir = CommonFSUtils.getNamespaceDir(tempDir,
+      tableName.getNamespaceAsString());
     final FileSystem fs = masterFileSystem.getFileSystem();
 
-    // move the table to the temporary directory
+    // checks the temporary directory does not exist
-    if (!fs.rename(tableDir, tempTableDir)) {
+    assertFalse(fs.exists(tempNsDir));
-      fail();
-    }
-
-    masterFileSystem.checkTempDir(tempDir, UTIL.getConfiguration(), fs);
-
-    // check if the temporary directory exists and is empty
-    assertTrue(fs.exists(tempDir));
-    assertEquals(0, fs.listStatus(tempDir).length);
-
-    // check for the existence of the archive directory
-    for (HRegion region : regions) {
-      Path archiveDir = HFileArchiveTestingUtil.getRegionArchiveDir(UTIL.getConfiguration(),
-        region);
-      assertTrue(fs.exists(archiveDir));
-    }
 
     UTIL.deleteTable(tableName);
   }

hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDeleteTableProcedure.java

@@ -17,34 +17,23 @@
  */
 package org.apache.hadoop.hbase.master.procedure;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
 import java.util.ArrayList;
 import java.util.List;
-import org.apache.hadoop.fs.FileSystem;
-import org.apache.hadoop.fs.FileUtil;
-import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.TableNotDisabledException;
 import org.apache.hadoop.hbase.TableNotFoundException;
 import org.apache.hadoop.hbase.client.RegionInfo;
-import org.apache.hadoop.hbase.client.Table;
-import org.apache.hadoop.hbase.master.MasterFileSystem;
 import org.apache.hadoop.hbase.procedure2.Procedure;
 import org.apache.hadoop.hbase.procedure2.ProcedureExecutor;
 import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility;
-import org.apache.hadoop.hbase.regionserver.HRegion;
 import org.apache.hadoop.hbase.testclassification.MasterTests;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
 import org.apache.hadoop.hbase.util.Bytes;
-import org.apache.hadoop.hbase.util.CommonFSUtils;
-import org.apache.hadoop.hbase.util.HFileArchiveTestingUtil;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
@@ -186,59 +175,4 @@ public class TestDeleteTableProcedure extends TestTableDDLProcedureBase {
 
     MasterProcedureTestingUtility.validateTableDeletion(getMaster(), tableName);
   }
-
-  @Test
-  public void testDeleteWhenTempDirIsNotEmpty() throws Exception {
-    final TableName tableName = TableName.valueOf(name.getMethodName());
-    final String FAM = "fam";
-    final byte[][] splitKeys = new byte[][] {
-      Bytes.toBytes("b"), Bytes.toBytes("c"), Bytes.toBytes("d")
-    };
-
-    // create the table
-    MasterProcedureTestingUtility.createTable(
-      getMasterProcedureExecutor(), tableName, splitKeys, FAM);
-
-    // get the current store files for the regions
-    List<HRegion> regions = UTIL.getHBaseCluster().getRegions(tableName);
-    // make sure we have 4 regions serving this table
-    assertEquals(4, regions.size());
-
-    // load the table
-    try (Table table = UTIL.getConnection().getTable(tableName)) {
-      UTIL.loadTable(table, Bytes.toBytes(FAM));
-    }
-
-    // disable the table so that we can manipulate the files
-    UTIL.getAdmin().disableTable(tableName);
-
-    final MasterFileSystem masterFileSystem =
-      UTIL.getMiniHBaseCluster().getMaster().getMasterFileSystem();
-    final Path tableDir = CommonFSUtils.getTableDir(masterFileSystem.getRootDir(), tableName);
-    final Path tempDir = masterFileSystem.getTempDir();
-    final Path tempTableDir = CommonFSUtils.getTableDir(tempDir, tableName);
-    final FileSystem fs = masterFileSystem.getFileSystem();
-
-    // copy the table to the temporary directory to make sure the temp directory is not empty
-    if (!FileUtil.copy(fs, tableDir, fs, tempTableDir, false, UTIL.getConfiguration())) {
-      fail();
-    }
-
-    // delete the table
-    final ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
-    long procId = ProcedureTestingUtility.submitAndWait(procExec,
-      new DeleteTableProcedure(procExec.getEnvironment(), tableName));
-    ProcedureTestingUtility.assertProcNotFailed(procExec, procId);
-    MasterProcedureTestingUtility.validateTableDeletion(getMaster(), tableName);
-
-    // check if the temporary directory is deleted
-    assertFalse(fs.exists(tempTableDir));
-
-    // check for the existence of the archive directory
-    for (HRegion region : regions) {
-      Path archiveDir = HFileArchiveTestingUtil.getRegionArchiveDir(UTIL.getConfiguration(),
-        region);
-      assertTrue(fs.exists(archiveDir));
-    }
-  }
 }