Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 19, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5847e93
HBASE-26454 CreateTableProcedure still relies on temp dir and renames…
Nov 12, 2021
22b207f
adjusting TestMasterFileSystem UTs
Nov 16, 2021
c2a4db6
fixing UT failure
Nov 18, 2021
97f4969
fixing checkstyles and whitespaces
Nov 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 5 additions & 25 deletions ...e-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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),
Copy link
Contributor Author

@wchevreuil wchevreuil Nov 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found out this was creating table tmp dir on CreateTableProcedure.CREATE_TABLE_POST_OPERATION when ACL is enabled.

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
Open in desktop
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));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now make sure no temp dir was created for table namespace during the CreateTableProcedure run.


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
Open in desktop
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));
}
}
}