Skip to content

Commit d482b64

Browse files
author
Prathyusha Garre
committed
HBASE-29662 - Avoid regionDir/tableDir creation as part of .regioninfo file creation in HRegion initialize
1 parent 47f7e1d commit d482b64

File tree

4 files changed

+143
-8
lines changed

4 files changed

+143
-8
lines changed

hbase-common/src/main/java/org/apache/hadoop/hbase/util/CommonFSUtils.java

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,11 +187,40 @@ public static int getDefaultBufferSize(final FileSystem fs) {
187187
*/
188188
public static FSDataOutputStream create(FileSystem fs, Path path, FsPermission perm,
189189
boolean overwrite) throws IOException {
190+
return create(fs, path, perm, overwrite, true);
191+
}
192+
193+
/**
194+
* Create the specified file on the filesystem. By default, this will:
195+
* <ol>
196+
* <li>apply the umask in the configuration (if it is enabled)</li>
197+
* <li>use the fs configured buffer size (or 4096 if not set)</li>
198+
* <li>use the default replication</li>
199+
* <li>use the default block size</li>
200+
* <li>not track progress</li>
201+
* </ol>
202+
* @param fs {@link FileSystem} on which to write the file
203+
* @param path {@link Path} to the file to write
204+
* @param perm intial permissions
205+
* @param overwrite Whether or not the created file should be overwritten.
206+
* @param isRecursiveCreate recursively create parent directories
207+
* @return output stream to the created file
208+
* @throws IOException if the file cannot be created
209+
*/
210+
public static FSDataOutputStream create(FileSystem fs, Path path, FsPermission perm,
211+
boolean overwrite, boolean isRecursiveCreate) throws IOException {
190212
if (LOG.isTraceEnabled()) {
191-
LOG.trace("Creating file={} with permission={}, overwrite={}", path, perm, overwrite);
213+
LOG.trace("Creating file={} with permission={}, overwrite={}, recursive={}", path, perm,
214+
overwrite, isRecursiveCreate);
215+
}
216+
217+
if (isRecursiveCreate) {
218+
return fs.create(path, perm, overwrite, getDefaultBufferSize(fs),
219+
getDefaultReplication(fs, path), getDefaultBlockSize(fs, path), null);
220+
} else {
221+
return fs.createNonRecursive(path, perm, overwrite, getDefaultBufferSize(fs),
222+
getDefaultReplication(fs, path), getDefaultBlockSize(fs, path), null);
192223
}
193-
return fs.create(path, perm, overwrite, getDefaultBufferSize(fs),
194-
getDefaultReplication(fs, path), getDefaultBlockSize(fs, path), null);
195224
}
196225

197226
/**

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -764,7 +764,10 @@ private static void writeRegionInfoFileContent(final Configuration conf, final F
764764
// First check to get the permissions
765765
FsPermission perms = CommonFSUtils.getFilePermissions(fs, conf, HConstants.DATA_FILE_UMASK_KEY);
766766
// Write the RegionInfo file content
767-
try (FSDataOutputStream out = FSUtils.create(conf, fs, regionInfoFile, perms, null)) {
767+
// HBASE-29662: Fail .regioninfo file creation, if the region directory doesn't exist,
768+
// avoiding silent masking of missing region directories during region initialization.
769+
// The region directory should already exist when this method is called.
770+
try (FSDataOutputStream out = FSUtils.create(conf, fs, regionInfoFile, perms, null, false)) {
768771
out.write(content);
769772
}
770773
}

hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,14 +212,40 @@ public static boolean deleteRegionDir(final Configuration conf, final RegionInfo
212212
*/
213213
public static FSDataOutputStream create(Configuration conf, FileSystem fs, Path path,
214214
FsPermission perm, InetSocketAddress[] favoredNodes) throws IOException {
215+
return create(conf, fs, path, perm, favoredNodes, true);
216+
}
217+
218+
/**
219+
* Create the specified file on the filesystem. By default, this will:
220+
* <ol>
221+
* <li>overwrite the file if it exists</li>
222+
* <li>apply the umask in the configuration (if it is enabled)</li>
223+
* <li>use the fs configured buffer size (or 4096 if not set)</li>
224+
* <li>use the configured column family replication or default replication if
225+
* {@link ColumnFamilyDescriptorBuilder#DEFAULT_DFS_REPLICATION}</li>
226+
* <li>use the default block size</li>
227+
* <li>not track progress</li>
228+
* </ol>
229+
* @param conf configurations
230+
* @param fs {@link FileSystem} on which to write the file
231+
* @param path {@link Path} to the file to write
232+
* @param perm permissions
233+
* @param favoredNodes favored data nodes
234+
* @param isRecursiveCreate recursively create parent directories
235+
* @return output stream to the created file
236+
* @throws IOException if the file cannot be created
237+
*/
238+
public static FSDataOutputStream create(Configuration conf, FileSystem fs, Path path,
239+
FsPermission perm, InetSocketAddress[] favoredNodes, boolean isRecursiveCreate)
240+
throws IOException {
215241
if (fs instanceof HFileSystem) {
216242
FileSystem backingFs = ((HFileSystem) fs).getBackingFs();
217243
if (backingFs instanceof DistributedFileSystem) {
218244
short replication = Short.parseShort(conf.get(ColumnFamilyDescriptorBuilder.DFS_REPLICATION,
219245
String.valueOf(ColumnFamilyDescriptorBuilder.DEFAULT_DFS_REPLICATION)));
220-
DistributedFileSystem.HdfsDataOutputStreamBuilder builder =
221-
((DistributedFileSystem) backingFs).createFile(path).recursive().permission(perm)
222-
.create();
246+
DistributedFileSystem.HdfsDataOutputStreamBuilder builder = null;
247+
builder = ((DistributedFileSystem) backingFs).createFile(path).recursive().permission(perm)
248+
.create();
223249
if (favoredNodes != null) {
224250
builder.favoredNodes(favoredNodes);
225251
}
@@ -230,7 +256,7 @@ public static FSDataOutputStream create(Configuration conf, FileSystem fs, Path
230256
}
231257

232258
}
233-
return CommonFSUtils.create(fs, path, perm, true);
259+
return CommonFSUtils.create(fs, path, perm, true, isRecursiveCreate);
234260
}
235261

236262
/**

hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionFileSystem.java

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@
4343
import org.apache.hadoop.hbase.client.RegionInfo;
4444
import org.apache.hadoop.hbase.client.RegionInfoBuilder;
4545
import org.apache.hadoop.hbase.client.Table;
46+
import org.apache.hadoop.hbase.client.TableDescriptor;
47+
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
4648
import org.apache.hadoop.hbase.fs.HFileSystem;
4749
import org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTracker;
4850
import org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory;
@@ -51,6 +53,7 @@
5153
import org.apache.hadoop.hbase.util.Bytes;
5254
import org.apache.hadoop.hbase.util.CommonFSUtils;
5355
import org.apache.hadoop.hbase.util.FSUtils;
56+
import org.apache.hadoop.hbase.wal.WAL;
5457
import org.apache.hadoop.util.Progressable;
5558
import org.junit.ClassRule;
5659
import org.junit.Rule;
@@ -387,4 +390,78 @@ public void testTempAndCommit() throws IOException {
387390

388391
fs.delete(rootDir, true);
389392
}
393+
394+
/**
395+
* Test for HBASE-29662: HRegion.initialize() should fail when trying to recreate .regioninfo file
396+
* after the region directory has been deleted. This validates that .regioninfo file creation does
397+
* not create parent directories recursively.
398+
*/
399+
@Test
400+
public void testHRegionInitializeFailsWithDeletedRegionDir() throws Exception {
401+
LOG.info("Testing HRegion initialize failure with deleted region directory");
402+
403+
TEST_UTIL = new HBaseTestingUtil();
404+
Configuration conf = TEST_UTIL.getConfiguration();
405+
Path testDir = TEST_UTIL.getDataTestDir("testHRegionInitFailure");
406+
FileSystem fs = testDir.getFileSystem(conf);
407+
408+
// Create table descriptor
409+
TableName tableName = TableName.valueOf("TestHRegionInitWithDeletedDir");
410+
byte[] family = Bytes.toBytes("info");
411+
TableDescriptor htd = TableDescriptorBuilder.newBuilder(tableName)
412+
.setColumnFamily(ColumnFamilyDescriptorBuilder.of(family)).build();
413+
414+
// Create region info
415+
RegionInfo regionInfo =
416+
RegionInfoBuilder.newBuilder(tableName).setStartKey(null).setEndKey(null).build();
417+
418+
Path tableDir = CommonFSUtils.getTableDir(testDir, tableName);
419+
420+
// Create WAL for the region
421+
WAL wal = HBaseTestingUtil.createWal(conf, testDir, regionInfo);
422+
423+
try {
424+
// Create region normally (this should succeed and create region directory)
425+
LOG.info("Creating region normally - should succeed");
426+
HRegion region = HRegion.createHRegion(regionInfo, testDir, conf, htd, wal, true);
427+
428+
// Verify region directory exists
429+
Path regionDir = new Path(tableDir, regionInfo.getEncodedName());
430+
assertTrue("Region directory should exist after creation", fs.exists(regionDir));
431+
432+
Path regionInfoFile = new Path(regionDir, HRegionFileSystem.REGION_INFO_FILE);
433+
assertTrue("Region info file should exist after creation", fs.exists(regionInfoFile));
434+
435+
// Delete the region directory (simulating external deletion or corruption)
436+
assertTrue(fs.delete(regionDir, true));
437+
assertFalse("Region directory should not exist after deletion", fs.exists(regionDir));
438+
439+
// Try to open/initialize the region again - this should fail
440+
LOG.info("Attempting to re-initialize region with deleted directory - should fail");
441+
442+
// Create a new region instance (simulating region server restart or reopen)
443+
HRegion newRegion = HRegion.newHRegion(tableDir, wal, fs, conf, regionInfo, htd, null);
444+
445+
// Try to initialize - this should fail because the regionDir doesn't exist
446+
IOException exception = null;
447+
try {
448+
newRegion.initialize(null);
449+
} catch (IOException e) {
450+
exception = e;
451+
}
452+
453+
// Verify the exception is related to missing parent directory
454+
assertNotNull("Exception should be thrown", exception);
455+
String exceptionMessage = exception.getMessage().toLowerCase();
456+
assertTrue(exceptionMessage.contains("parent directory doesn't exist"));
457+
assertFalse("Region directory should still not exist after failed initialization",
458+
fs.exists(regionDir));
459+
460+
} finally {
461+
if (wal != null) {
462+
wal.close();
463+
}
464+
TEST_UTIL.cleanupTestDir();
465+
}
466+
}
390467
}

0 commit comments

Comments
 (0)