Skip to content

Commit d00b5fa

Browse files
wchevreuiljoshelser
wchevreuil
authored and
joshelser
committed
HBASE-26454 CreateTableProcedure still relies on temp dir and renames… (#3845)
Signed-off-by: Duo Zhang <[email protected]>
1 parent a288365 commit d00b5fa

File tree

5 files changed

+43
-181
lines changed

5 files changed

+43
-181
lines changed

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

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import java.util.ArrayList;
2424
import java.util.List;
2525
import java.util.function.Supplier;
26-
import org.apache.hadoop.fs.FileSystem;
2726
import org.apache.hadoop.fs.Path;
2827
import org.apache.hadoop.hbase.DoNotRetryIOException;
2928
import org.apache.hadoop.hbase.HBaseIOException;
@@ -336,41 +335,22 @@ protected static List<RegionInfo> createFsLayout(final MasterProcedureEnv env,
336335
final TableDescriptor tableDescriptor, List<RegionInfo> newRegions,
337336
final CreateHdfsRegions hdfsRegionHandler) throws IOException {
338337
final MasterFileSystem mfs = env.getMasterServices().getMasterFileSystem();
339-
final Path tempdir = mfs.getTempDir();
340338

341339
// 1. Create Table Descriptor
342340
// using a copy of descriptor, table will be created enabling first
343-
final Path tempTableDir = CommonFSUtils.getTableDir(tempdir, tableDescriptor.getTableName());
341+
final Path tableDir = CommonFSUtils.getTableDir(mfs.getRootDir(),
342+
tableDescriptor.getTableName());
344343
((FSTableDescriptors)(env.getMasterServices().getTableDescriptors()))
345-
.createTableDescriptorForTableDirectory(tempTableDir, tableDescriptor, false);
344+
.createTableDescriptorForTableDirectory(
345+
tableDir, tableDescriptor, false);
346346

347347
// 2. Create Regions
348-
newRegions = hdfsRegionHandler.createHdfsRegions(env, tempdir,
348+
newRegions = hdfsRegionHandler.createHdfsRegions(env, mfs.getRootDir(),
349349
tableDescriptor.getTableName(), newRegions);
350350

351-
// 3. Move Table temp directory to the hbase root location
352-
moveTempDirectoryToHBaseRoot(env, tableDescriptor, tempTableDir);
353-
354351
return newRegions;
355352
}
356353

357-
protected static void moveTempDirectoryToHBaseRoot(
358-
final MasterProcedureEnv env,
359-
final TableDescriptor tableDescriptor,
360-
final Path tempTableDir) throws IOException {
361-
final MasterFileSystem mfs = env.getMasterServices().getMasterFileSystem();
362-
final Path tableDir =
363-
CommonFSUtils.getTableDir(mfs.getRootDir(), tableDescriptor.getTableName());
364-
FileSystem fs = mfs.getFileSystem();
365-
if (!fs.delete(tableDir, true) && fs.exists(tableDir)) {
366-
throw new IOException("Couldn't delete " + tableDir);
367-
}
368-
if (!fs.rename(tempTableDir, tableDir)) {
369-
throw new IOException("Unable to move table from temp=" + tempTableDir +
370-
" to hbase root=" + tableDir);
371-
}
372-
}
373-
374354
protected static List<RegionInfo> addTableToMeta(final MasterProcedureEnv env,
375355
final TableDescriptor tableDescriptor, final List<RegionInfo> regions) throws IOException {
376356
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‎

Lines changed: 30 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,8 @@
2020

2121
import java.io.IOException;
2222
import java.util.ArrayList;
23-
import java.util.Arrays;
2423
import java.util.List;
2524
import java.util.stream.Collectors;
26-
import org.apache.hadoop.fs.FileStatus;
2725
import org.apache.hadoop.fs.FileSystem;
2826
import org.apache.hadoop.fs.Path;
2927
import org.apache.hadoop.hbase.MetaTableAccessor;
@@ -277,82 +275,49 @@ protected static void deleteFromFs(final MasterProcedureEnv env,
277275
final boolean archive) throws IOException {
278276
final MasterFileSystem mfs = env.getMasterServices().getMasterFileSystem();
279277
final FileSystem fs = mfs.getFileSystem();
280-
final Path tempdir = mfs.getTempDir();
281278

282279
final Path tableDir = CommonFSUtils.getTableDir(mfs.getRootDir(), tableName);
283-
final Path tempTableDir = CommonFSUtils.getTableDir(tempdir, tableName);
284280

285281
if (fs.exists(tableDir)) {
286-
// Ensure temp exists
287-
if (!fs.exists(tempdir) && !fs.mkdirs(tempdir)) {
288-
throw new IOException("HBase temp directory '" + tempdir + "' creation failure.");
289-
}
290-
291-
// Ensure parent exists
292-
if (!fs.exists(tempTableDir.getParent()) && !fs.mkdirs(tempTableDir.getParent())) {
293-
throw new IOException("HBase temp directory '" + tempdir + "' creation failure.");
294-
}
295-
296-
if (fs.exists(tempTableDir)) {
297-
// TODO
298-
// what's in this dir? something old? probably something manual from the user...
299-
// let's get rid of this stuff...
300-
FileStatus[] files = fs.listStatus(tempTableDir);
301-
if (files != null && files.length > 0) {
302-
List<Path> regionDirList = Arrays.stream(files)
303-
.filter(FileStatus::isDirectory)
304-
.map(FileStatus::getPath)
305-
.collect(Collectors.toList());
306-
HFileArchiver.archiveRegions(env.getMasterConfiguration(), fs, mfs.getRootDir(),
307-
tempTableDir, regionDirList);
282+
// Archive regions from FS (temp directory)
283+
if (archive) {
284+
List<Path> regionDirList = regions.stream().filter(RegionReplicaUtil::isDefaultReplica)
285+
.map(region ->
286+
FSUtils.getRegionDirFromTableDir(tableDir, region)).collect(Collectors.toList());
287+
HFileArchiver
288+
.archiveRegions(env.getMasterConfiguration(), fs, mfs.getRootDir(), tableDir,
289+
regionDirList);
290+
if (!regionDirList.isEmpty()) {
291+
LOG.debug("Archived {} regions", tableName);
308292
}
309-
fs.delete(tempTableDir, true);
310293
}
311294

312-
// Move the table in /hbase/.tmp
313-
if (!fs.rename(tableDir, tempTableDir)) {
314-
throw new IOException("Unable to move '" + tableDir + "' to temp '" + tempTableDir + "'");
295+
// Archive mob data
296+
Path mobTableDir =
297+
CommonFSUtils.getTableDir(new Path(mfs.getRootDir(), MobConstants.MOB_DIR_NAME), tableName);
298+
Path regionDir = new Path(mobTableDir, MobUtils.getMobRegionInfo(tableName).getEncodedName());
299+
if (fs.exists(regionDir)) {
300+
HFileArchiver.archiveRegion(fs, mfs.getRootDir(), mobTableDir, regionDir);
315301
}
316-
}
317302

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

330-
// Archive mob data
331-
Path mobTableDir =
332-
CommonFSUtils.getTableDir(new Path(mfs.getRootDir(), MobConstants.MOB_DIR_NAME), tableName);
333-
Path regionDir =
334-
new Path(mobTableDir, MobUtils.getMobRegionInfo(tableName).getEncodedName());
335-
if (fs.exists(regionDir)) {
336-
HFileArchiver.archiveRegion(fs, mfs.getRootDir(), mobTableDir, regionDir);
337-
}
338-
339-
// Delete table directory from FS (temp directory)
340-
if (!fs.delete(tempTableDir, true) && fs.exists(tempTableDir)) {
341-
throw new IOException("Couldn't delete " + tempTableDir);
342-
}
343-
344-
// Delete the table directory where the mob files are saved
345-
if (mobTableDir != null && fs.exists(mobTableDir)) {
346-
if (!fs.delete(mobTableDir, true)) {
347-
throw new IOException("Couldn't delete mob dir " + mobTableDir);
308+
// Delete the table directory where the mob files are saved
309+
if (mobTableDir != null && fs.exists(mobTableDir)) {
310+
if (!fs.delete(mobTableDir, true)) {
311+
throw new IOException("Couldn't delete mob dir " + mobTableDir);
312+
}
348313
}
349-
}
350314

351-
// Delete the directory on wal filesystem
352-
FileSystem walFs = mfs.getWALFileSystem();
353-
Path tableWALDir = CommonFSUtils.getWALTableDir(env.getMasterConfiguration(), tableName);
354-
if (walFs.exists(tableWALDir) && !walFs.delete(tableWALDir, true)) {
355-
throw new IOException("Couldn't delete table dir on wal filesystem" + tableWALDir);
315+
// Delete the directory on wal filesystem
316+
FileSystem walFs = mfs.getWALFileSystem();
317+
Path tableWALDir = CommonFSUtils.getWALTableDir(env.getMasterConfiguration(), tableName);
318+
if (walFs.exists(tableWALDir) && !walFs.delete(tableWALDir, true)) {
319+
throw new IOException("Couldn't delete table dir on wal filesystem" + tableWALDir);
320+
}
356321
}
357322
}
358323

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -474,8 +474,8 @@ List<Path> getNamespaceRootPaths(String namespace) {
474474
*/
475475
List<Path> getTableRootPaths(TableName tableName, boolean includeSnapshotPath)
476476
throws IOException {
477-
List<Path> paths = Lists.newArrayList(pathHelper.getTmpTableDir(tableName),
478-
pathHelper.getDataTableDir(tableName), pathHelper.getMobTableDir(tableName),
477+
List<Path> paths = Lists.newArrayList(pathHelper.getDataTableDir(tableName),
478+
pathHelper.getMobTableDir(tableName),
479479
pathHelper.getArchiveTableDir(tableName));
480480
if (includeSnapshotPath) {
481481
paths.addAll(getTableSnapshotPaths(tableName));

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

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@
1818
package org.apache.hadoop.hbase.master;
1919

2020
import static org.junit.Assert.assertEquals;
21-
import static org.junit.Assert.assertTrue;
22-
import static org.junit.Assert.fail;
21+
import static org.junit.Assert.assertFalse;
2322

2423
import java.util.List;
2524
import org.apache.hadoop.fs.FileSystem;
@@ -33,7 +32,6 @@
3332
import org.apache.hadoop.hbase.testclassification.MediumTests;
3433
import org.apache.hadoop.hbase.util.Bytes;
3534
import org.apache.hadoop.hbase.util.CommonFSUtils;
36-
import org.apache.hadoop.hbase.util.HFileArchiveTestingUtil;
3735
import org.junit.AfterClass;
3836
import org.junit.BeforeClass;
3937
import org.junit.ClassRule;
@@ -85,7 +83,7 @@ public void testFsUriSetProperly() throws Exception {
8583
}
8684

8785
@Test
88-
public void testCheckTempDir() throws Exception {
86+
public void testCheckNoTempDir() throws Exception {
8987
final MasterFileSystem masterFileSystem =
9088
UTIL.getMiniHBaseCluster().getMaster().getMasterFileSystem();
9189

@@ -110,28 +108,13 @@ public void testCheckTempDir() throws Exception {
110108
// disable the table so that we can manipulate the files
111109
UTIL.getAdmin().disableTable(tableName);
112110

113-
final Path tableDir = CommonFSUtils.getTableDir(masterFileSystem.getRootDir(), tableName);
114111
final Path tempDir = masterFileSystem.getTempDir();
115-
final Path tempTableDir = CommonFSUtils.getTableDir(tempDir, tableName);
112+
final Path tempNsDir = CommonFSUtils.getNamespaceDir(tempDir,
113+
tableName.getNamespaceAsString());
116114
final FileSystem fs = masterFileSystem.getFileSystem();
117115

118-
// move the table to the temporary directory
119-
if (!fs.rename(tableDir, tempTableDir)) {
120-
fail();
121-
}
122-
123-
masterFileSystem.checkTempDir(tempDir, UTIL.getConfiguration(), fs);
124-
125-
// check if the temporary directory exists and is empty
126-
assertTrue(fs.exists(tempDir));
127-
assertEquals(0, fs.listStatus(tempDir).length);
128-
129-
// check for the existence of the archive directory
130-
for (HRegion region : regions) {
131-
Path archiveDir = HFileArchiveTestingUtil.getRegionArchiveDir(UTIL.getConfiguration(),
132-
region);
133-
assertTrue(fs.exists(archiveDir));
134-
}
116+
// checks the temporary directory does not exist
117+
assertFalse(fs.exists(tempNsDir));
135118

136119
UTIL.deleteTable(tableName);
137120
}

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

Lines changed: 0 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -17,34 +17,23 @@
1717
*/
1818
package org.apache.hadoop.hbase.master.procedure;
1919

20-
import static org.junit.Assert.assertEquals;
21-
import static org.junit.Assert.assertFalse;
2220
import static org.junit.Assert.assertNotNull;
2321
import static org.junit.Assert.assertNull;
2422
import static org.junit.Assert.assertTrue;
25-
import static org.junit.Assert.fail;
2623

2724
import java.util.ArrayList;
2825
import java.util.List;
29-
import org.apache.hadoop.fs.FileSystem;
30-
import org.apache.hadoop.fs.FileUtil;
31-
import org.apache.hadoop.fs.Path;
3226
import org.apache.hadoop.hbase.HBaseClassTestRule;
3327
import org.apache.hadoop.hbase.TableName;
3428
import org.apache.hadoop.hbase.TableNotDisabledException;
3529
import org.apache.hadoop.hbase.TableNotFoundException;
3630
import org.apache.hadoop.hbase.client.RegionInfo;
37-
import org.apache.hadoop.hbase.client.Table;
38-
import org.apache.hadoop.hbase.master.MasterFileSystem;
3931
import org.apache.hadoop.hbase.procedure2.Procedure;
4032
import org.apache.hadoop.hbase.procedure2.ProcedureExecutor;
4133
import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility;
42-
import org.apache.hadoop.hbase.regionserver.HRegion;
4334
import org.apache.hadoop.hbase.testclassification.MasterTests;
4435
import org.apache.hadoop.hbase.testclassification.MediumTests;
4536
import org.apache.hadoop.hbase.util.Bytes;
46-
import org.apache.hadoop.hbase.util.CommonFSUtils;
47-
import org.apache.hadoop.hbase.util.HFileArchiveTestingUtil;
4837
import org.junit.ClassRule;
4938
import org.junit.Rule;
5039
import org.junit.Test;
@@ -186,59 +175,4 @@ public void testRecoveryAndDoubleExecution() throws Exception {
186175

187176
MasterProcedureTestingUtility.validateTableDeletion(getMaster(), tableName);
188177
}
189-
190-
@Test
191-
public void testDeleteWhenTempDirIsNotEmpty() throws Exception {
192-
final TableName tableName = TableName.valueOf(name.getMethodName());
193-
final String FAM = "fam";
194-
final byte[][] splitKeys = new byte[][] {
195-
Bytes.toBytes("b"), Bytes.toBytes("c"), Bytes.toBytes("d")
196-
};
197-
198-
// create the table
199-
MasterProcedureTestingUtility.createTable(
200-
getMasterProcedureExecutor(), tableName, splitKeys, FAM);
201-
202-
// get the current store files for the regions
203-
List<HRegion> regions = UTIL.getHBaseCluster().getRegions(tableName);
204-
// make sure we have 4 regions serving this table
205-
assertEquals(4, regions.size());
206-
207-
// load the table
208-
try (Table table = UTIL.getConnection().getTable(tableName)) {
209-
UTIL.loadTable(table, Bytes.toBytes(FAM));
210-
}
211-
212-
// disable the table so that we can manipulate the files
213-
UTIL.getAdmin().disableTable(tableName);
214-
215-
final MasterFileSystem masterFileSystem =
216-
UTIL.getMiniHBaseCluster().getMaster().getMasterFileSystem();
217-
final Path tableDir = CommonFSUtils.getTableDir(masterFileSystem.getRootDir(), tableName);
218-
final Path tempDir = masterFileSystem.getTempDir();
219-
final Path tempTableDir = CommonFSUtils.getTableDir(tempDir, tableName);
220-
final FileSystem fs = masterFileSystem.getFileSystem();
221-
222-
// copy the table to the temporary directory to make sure the temp directory is not empty
223-
if (!FileUtil.copy(fs, tableDir, fs, tempTableDir, false, UTIL.getConfiguration())) {
224-
fail();
225-
}
226-
227-
// delete the table
228-
final ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
229-
long procId = ProcedureTestingUtility.submitAndWait(procExec,
230-
new DeleteTableProcedure(procExec.getEnvironment(), tableName));
231-
ProcedureTestingUtility.assertProcNotFailed(procExec, procId);
232-
MasterProcedureTestingUtility.validateTableDeletion(getMaster(), tableName);
233-
234-
// check if the temporary directory is deleted
235-
assertFalse(fs.exists(tempTableDir));
236-
237-
// check for the existence of the archive directory
238-
for (HRegion region : regions) {
239-
Path archiveDir = HFileArchiveTestingUtil.getRegionArchiveDir(UTIL.getConfiguration(),
240-
region);
241-
assertTrue(fs.exists(archiveDir));
242-
}
243-
}
244178
}

0 commit comments

Comments
 (0)