Skip to content

Commit

Permalink
HBASE-26454 CreateTableProcedure still relies on temp dir and renames… (
Browse files Browse the repository at this point in the history 
apache#3845)

Signed-off-by: Duo Zhang <zhangduo@apache.org>
  • Loading branch information
@wchevreuil @joshelser
wchevreuil authored and joshelser committed Dec 22, 2021
1 parent a288365 commit d00b5fa
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 181 deletions.
30 changes: 5 additions & 25 deletions ...e-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import java.util.ArrayList;
import java.util.List;
import java.util.function.Supplier;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.DoNotRetryIOException;
import org.apache.hadoop.hbase.HBaseIOException;
Expand Down Expand Up @@ -336,41 +335,22 @@ protected static List<RegionInfo> createFsLayout(final MasterProcedureEnv env,
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;
Expand Down
95 changes: 30 additions & 65 deletions ...e-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@

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;
Expand Down Expand Up @@ -277,82 +275,49 @@ protected static void deleteFromFs(final MasterProcedureEnv env,
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);
// Archive regions from FS (temp directory)
if (archive) {
List<Path> regionDirList = regions.stream().filter(RegionReplicaUtil::isDefaultReplica)
.map(region ->
FSUtils.getRegionDirFromTableDir(tableDir, region)).collect(Collectors.toList());
HFileArchiver
.archiveRegions(env.getMasterConfiguration(), fs, mfs.getRootDir(), tableDir,
regionDirList);
if (!regionDirList.isEmpty()) {
LOG.debug("Archived {} regions", tableName);
}
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 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);
}
}

// Archive regions from FS (temp directory)
if (archive) {
List<Path> regionDirList = regions.stream().filter(RegionReplicaUtil::isDefaultReplica)
.map(region -> FSUtils.getRegionDirFromTableDir(tempTableDir, region))
.collect(Collectors.toList());
HFileArchiver.archiveRegions(env.getMasterConfiguration(), fs, mfs.getRootDir(), tempTableDir,
regionDirList);
if (!regionDirList.isEmpty()) {
LOG.debug("Archived {} regions", tableName);
// Delete table directory from FS
if (!fs.delete(tableDir, true) && fs.exists(tableDir)) {
throw new IOException("Couldn't delete " + tableDir);
}
}

// 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 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);
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions ...r/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,8 +474,8 @@ List<Path> getNamespaceRootPaths(String namespace) {
*/
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));
Expand Down
29 changes: 6 additions & 23 deletions hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterFileSystem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -33,7 +32,6 @@
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;
Expand Down Expand Up @@ -85,7 +83,7 @@ public void testFsUriSetProperly() throws Exception {
}

@Test
public void testCheckTempDir() throws Exception {
public void testCheckNoTempDir() throws Exception {
final MasterFileSystem masterFileSystem =
UTIL.getMiniHBaseCluster().getMaster().getMasterFileSystem();

Expand All @@ -110,28 +108,13 @@ public void testCheckTempDir() throws Exception {
// 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);
}
Expand Down
66 changes: 0 additions & 66 deletions ...rver/src/test/java/org/apache/hadoop/hbase/master/procedure/TestDeleteTableProcedure.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -186,59 +175,4 @@ public void testRecoveryAndDoubleExecution() throws Exception {

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));
}
}
}

0 comments on commit d00b5fa

Please sign in to comment.