You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zh...@apache.org on 2022/01/01 15:56:13 UTC
[hbase] 14/15: HBASE-26454 CreateTableProcedure still relies on temp dir and renames… (#3845)
This is an automated email from the ASF dual-hosted git repository.
zhangduo pushed a commit to branch HBASE-26067-branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git
commit 79fb093ba1caf2564f7a0dde36ce92eedd514755
Author: Wellington Ramos Chevreuil <wc...@apache.org>
AuthorDate: Fri Nov 19 12:16:29 2021 +0000
HBASE-26454 CreateTableProcedure still relies on temp dir and renames… (#3845)
Signed-off-by: Duo Zhang <zh...@apache.org>
---
.../master/procedure/CreateTableProcedure.java | 30 +-----
.../master/procedure/DeleteTableProcedure.java | 115 +++++++--------------
.../access/SnapshotScannerHDFSAclHelper.java | 4 +-
.../hadoop/hbase/master/TestMasterFileSystem.java | 29 ++----
.../master/procedure/TestDeleteTableProcedure.java | 66 ------------
5 files changed, 53 insertions(+), 191 deletions(-)
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java
index 55e3212..441fddb 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java
+++ b/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;
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java
index 8322383..4614486 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java
+++ b/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,92 +276,59 @@ 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.");
+ // 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(tableDir, region));
+ List<RegionInfo> mergeRegions = MetaTableAccessor
+ .getMergeRegions(env.getMasterServices().getConnection(), region.getRegionName());
+ if (!CollectionUtils.isEmpty(mergeRegions)) {
+ mergeRegions.stream().forEach(
+ r -> regionDirList.add(FSUtils.getRegionDirFromTableDir(tableDir, r)));
+ }
+ }
+ }
+ HFileArchiver
+ .archiveRegions(env.getMasterConfiguration(), fs, mfs.getRootDir(), tableDir,
+ regionDirList);
+ if (!regionDirList.isEmpty()) {
+ LOG.debug("Archived {} regions", tableName);
+ }
}
- 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);
+ // Archive mob data
+ Path mobTableDir =
+ CommonFSUtils.getTableDir(new Path(mfs.getRootDir(), MobConstants.MOB_DIR_NAME), tableName);
+ Path regionDir = new Path(mobTableDir, MobUtils.getMobRegionInfo(tableName).getEncodedName());
+ if (fs.exists(regionDir)) {
+ HFileArchiver.archiveRegion(fs, mfs.getRootDir(), mobTableDir, regionDir);
}
- // Move the table in /hbase/.tmp
- if (!fs.rename(tableDir, tempTableDir)) {
- throw new IOException("Unable to move '" + tableDir + "' to temp '" + tempTableDir + "'");
+ // Delete table directory from FS
+ if (!fs.delete(tableDir, true) && fs.exists(tableDir)) {
+ throw new IOException("Couldn't delete " + tableDir);
}
- }
- // 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));
- List<RegionInfo> mergeRegions = MetaTableAccessor
- .getMergeRegions(env.getMasterServices().getConnection(), region.getRegionName());
- if (!CollectionUtils.isEmpty(mergeRegions)) {
- mergeRegions.stream()
- .forEach(r -> regionDirList.add(FSUtils.getRegionDirFromTableDir(tempTableDir, r)));
- }
+ // Delete the table directory where the mob files are saved
+ if (mobTableDir != null && fs.exists(mobTableDir)) {
+ if (!fs.delete(mobTableDir, true)) {
+ throw new IOException("Couldn't delete mob dir " + mobTableDir);
}
}
- HFileArchiver.archiveRegions(env.getMasterConfiguration(), fs, mfs.getRootDir(), tempTableDir,
- regionDirList);
- if (!regionDirList.isEmpty()) {
- LOG.debug("Archived {} regions", tableName);
- }
- }
-
- // Archive mob data
- Path mobTableDir =
- CommonFSUtils.getTableDir(new Path(mfs.getRootDir(), MobConstants.MOB_DIR_NAME), tableName);
- Path regionDir =
- 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)
- if (!fs.delete(tempTableDir, true) && fs.exists(tempTableDir)) {
- throw new IOException("Couldn't delete " + tempTableDir);
- }
- // Delete the table directory where the mob files are saved
- if (mobTableDir != null && fs.exists(mobTableDir)) {
- if (!fs.delete(mobTableDir, true)) {
- throw new IOException("Couldn't delete mob dir " + mobTableDir);
+ // Delete the directory on wal filesystem
+ FileSystem walFs = mfs.getWALFileSystem();
+ Path tableWALDir = CommonFSUtils.getWALTableDir(env.getMasterConfiguration(), tableName);
+ if (walFs.exists(tableWALDir) && !walFs.delete(tableWALDir, true)) {
+ throw new IOException("Couldn't delete table dir on wal filesystem" + tableWALDir);
}
}
-
- // Delete the directory on wal filesystem
- FileSystem walFs = mfs.getWALFileSystem();
- Path tableWALDir = CommonFSUtils.getWALTableDir(env.getMasterConfiguration(), tableName);
- if (walFs.exists(tableWALDir) && !walFs.delete(tableWALDir, true)) {
- throw new IOException("Couldn't delete table dir on wal filesystem" + tableWALDir);
- }
}
/**
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclHelper.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclHelper.java
index ffe8dab..fbdc638 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclHelper.java
+++ b/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),
- pathHelper.getDataTableDir(tableName), pathHelper.getMobTableDir(tableName),
+ List<Path> paths = Lists.newArrayList(pathHelper.getDataTableDir(tableName),
+ pathHelper.getMobTableDir(tableName),
pathHelper.getArchiveTableDir(tableName));
if (includeSnapshotPath) {
paths.addAll(getTableSnapshotPaths(tableName));
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterFileSystem.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterFileSystem.java
index 63d303d..1461c06 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterFileSystem.java
+++ b/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.fail;
+import static org.junit.Assert.assertFalse;
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
- if (!fs.rename(tableDir, tempTableDir)) {
- 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));
- }
+ // checks the temporary directory does not exist
+ assertFalse(fs.exists(tempNsDir));
UTIL.deleteTable(tableName);
}
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDeleteTableProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDeleteTableProcedure.java
index 1dd7dc4..9367a57 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDeleteTableProcedure.java
+++ b/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));
- }
- }
}