Skip to content

Commit 7b58903

Browse files
gvprathyusha6apurtell
gvprathyusha6
authored and
apurtell
committed
HBASE-29662 - Avoid regionDir/tableDir creation as part of .regioninfo file creation in HRegion initialize (#7406)
Signed-off-by: Andrew Purtell <[email protected]> Signed-off-by: Viraj Jasani <[email protected]> Conflicts: hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/MetaFixer.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TruncateRegionProcedure.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
1 parent 52c69cd commit 7b58903

File tree

13 files changed

+308
-16
lines changed

13 files changed

+308
-16
lines changed

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

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

198226
/**

‎hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableSnapshotInputFormat.java‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import org.apache.hadoop.hbase.client.TestTableSnapshotScanner;
4747
import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
4848
import org.apache.hadoop.hbase.mapreduce.TableSnapshotInputFormat.TableSnapshotRegionSplit;
49+
import org.apache.hadoop.hbase.snapshot.RestoreSnapshotHelper;
4950
import org.apache.hadoop.hbase.snapshot.SnapshotTestingUtils;
5051
import org.apache.hadoop.hbase.testclassification.LargeTests;
5152
import org.apache.hadoop.hbase.testclassification.VerySlowMapReduceTests;
@@ -583,4 +584,104 @@ public void testCleanRestoreDir() throws Exception {
583584
TableSnapshotInputFormat.cleanRestoreDir(job, snapshotName);
584585
Assert.assertFalse(fs.exists(restorePath));
585586
}
587+
588+
/**
589+
* Test that explicitly restores a snapshot to a temp directory and reads the restored regions via
590+
* ClientSideRegionScanner through a MapReduce job.
591+
* <p>
592+
* This test verifies the full workflow: 1. Create and load a table with data 2. Create a snapshot
593+
* and restore the snapshot to a temporary directory 3. Configure a job to read the restored
594+
* regions via ClientSideRegionScanner using TableSnapshotInputFormat and verify that it succeeds
595+
* 4. Delete restored temporary directory 5. Configure a new job and verify that it fails
596+
*/
597+
@Test
598+
public void testReadFromRestoredSnapshotViaMR() throws Exception {
599+
final TableName tableName = TableName.valueOf(name.getMethodName());
600+
final String snapshotName = tableName + "_snapshot";
601+
try {
602+
if (UTIL.getAdmin().tableExists(tableName)) {
603+
UTIL.deleteTable(tableName);
604+
}
605+
UTIL.createTable(tableName, FAMILIES, new byte[][] { bbb, yyy });
606+
607+
Admin admin = UTIL.getAdmin();
608+
int regionNum = admin.getRegions(tableName).size();
609+
LOG.info("Created table with {} regions", regionNum);
610+
611+
Table table = UTIL.getConnection().getTable(tableName);
612+
UTIL.loadTable(table, FAMILIES);
613+
table.close();
614+
615+
Path rootDir = CommonFSUtils.getRootDir(UTIL.getConfiguration());
616+
FileSystem fs = rootDir.getFileSystem(UTIL.getConfiguration());
617+
SnapshotTestingUtils.createSnapshotAndValidate(admin, tableName, Arrays.asList(FAMILIES),
618+
null, snapshotName, rootDir, fs, true);
619+
Path tempRestoreDir = UTIL.getDataTestDirOnTestFS("restore_" + snapshotName);
620+
RestoreSnapshotHelper.copySnapshotForScanner(UTIL.getConfiguration(), fs, rootDir,
621+
tempRestoreDir, snapshotName);
622+
Assert.assertTrue("Restore directory should exist", fs.exists(tempRestoreDir));
623+
624+
Job job = Job.getInstance(UTIL.getConfiguration());
625+
job.setJarByClass(TestTableSnapshotInputFormat.class);
626+
TableMapReduceUtil.addDependencyJarsForClasses(job.getConfiguration(),
627+
TestTableSnapshotInputFormat.class);
628+
Scan scan = new Scan().withStartRow(getStartRow()).withStopRow(getEndRow());
629+
Configuration conf = job.getConfiguration();
630+
conf.set("hbase.TableSnapshotInputFormat.snapshot.name", snapshotName);
631+
conf.set("hbase.TableSnapshotInputFormat.restore.dir", tempRestoreDir.toString());
632+
conf.setInt("hbase.mapreduce.splits.per.region", 1);
633+
job.setReducerClass(TestTableSnapshotReducer.class);
634+
job.setNumReduceTasks(1);
635+
job.setOutputFormatClass(NullOutputFormat.class);
636+
TableMapReduceUtil.initTableMapperJob(snapshotName, // table name (snapshot name in this case)
637+
scan, TestTableSnapshotMapper.class, ImmutableBytesWritable.class, NullWritable.class, job,
638+
false, false, TableSnapshotInputFormat.class);
639+
TableMapReduceUtil.resetCacheConfig(conf);
640+
Assert.assertTrue(job.waitForCompletion(true));
641+
Assert.assertTrue(job.isSuccessful());
642+
643+
// Now verify that job fails when restore directory is deleted
644+
Assert.assertTrue(fs.delete(tempRestoreDir, true));
645+
Assert.assertFalse("Restore directory should not exist after deletion",
646+
fs.exists(tempRestoreDir));
647+
Job failureJob = Job.getInstance(UTIL.getConfiguration());
648+
failureJob.setJarByClass(TestTableSnapshotInputFormat.class);
649+
TableMapReduceUtil.addDependencyJarsForClasses(failureJob.getConfiguration(),
650+
TestTableSnapshotInputFormat.class);
651+
Configuration failureConf = failureJob.getConfiguration();
652+
// Configure job to use the deleted restore directory
653+
failureConf.set("hbase.TableSnapshotInputFormat.snapshot.name", snapshotName);
654+
failureConf.set("hbase.TableSnapshotInputFormat.restore.dir", tempRestoreDir.toString());
655+
failureConf.setInt("hbase.mapreduce.splits.per.region", 1);
656+
failureJob.setReducerClass(TestTableSnapshotReducer.class);
657+
failureJob.setNumReduceTasks(1);
658+
failureJob.setOutputFormatClass(NullOutputFormat.class);
659+
660+
TableMapReduceUtil.initTableMapperJob(snapshotName, scan, TestTableSnapshotMapper.class,
661+
ImmutableBytesWritable.class, NullWritable.class, failureJob, false, false,
662+
TableSnapshotInputFormat.class);
663+
TableMapReduceUtil.resetCacheConfig(failureConf);
664+
665+
Assert.assertFalse("Restore directory should not exist before job execution",
666+
fs.exists(tempRestoreDir));
667+
failureJob.waitForCompletion(true);
668+
669+
Assert.assertFalse("Job should fail since the restored snapshot directory is deleted",
670+
failureJob.isSuccessful());
671+
672+
} finally {
673+
try {
674+
if (UTIL.getAdmin().tableExists(tableName)) {
675+
UTIL.deleteTable(tableName);
676+
}
677+
} catch (Exception e) {
678+
LOG.warn("Error deleting table", e);
679+
}
680+
try {
681+
UTIL.getAdmin().deleteSnapshot(snapshotName);
682+
} catch (Exception e) {
683+
LOG.warn("Error deleting snapshot", e);
684+
}
685+
}
686+
}
586687
}

‎hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/MetaFixer.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.SortedSet;
3030
import java.util.TreeSet;
3131
import java.util.stream.Collectors;
32+
import org.apache.hadoop.fs.Path;
3233
import org.apache.hadoop.hbase.HBaseIOException;
3334
import org.apache.hadoop.hbase.HConstants;
3435
import org.apache.hadoop.hbase.MetaTableAccessor;
@@ -38,10 +39,13 @@
3839
import org.apache.hadoop.hbase.client.RegionReplicaUtil;
3940
import org.apache.hadoop.hbase.client.TableDescriptor;
4041
import org.apache.hadoop.hbase.exceptions.MergeRegionException;
42+
import org.apache.hadoop.hbase.master.MasterFileSystem;
4143
import org.apache.hadoop.hbase.master.MasterServices;
4244
import org.apache.hadoop.hbase.master.assignment.TransitRegionStateProcedure;
45+
import org.apache.hadoop.hbase.regionserver.HRegionFileSystem;
4346
import org.apache.hadoop.hbase.replication.ReplicationException;
4447
import org.apache.hadoop.hbase.util.Bytes;
48+
import org.apache.hadoop.hbase.util.CommonFSUtils;
4549
import org.apache.hadoop.hbase.util.Pair;
4650
import org.apache.hadoop.hbase.util.ServerRegionReplicaUtil;
4751
import org.apache.yetus.audience.InterfaceAudience;
@@ -105,6 +109,7 @@ void fixHoles(CatalogJanitorReport report) {
105109

106110
final List<RegionInfo> newRegionInfos = createRegionInfosForHoles(holes);
107111
final List<RegionInfo> newMetaEntries = createMetaEntries(masterServices, newRegionInfos);
112+
createRegionDirectories(masterServices, newMetaEntries);
108113
final TransitRegionStateProcedure[] assignProcedures =
109114
masterServices.getAssignmentManager().createRoundRobinAssignProcedures(newMetaEntries);
110115

@@ -226,6 +231,27 @@ private static List<RegionInfo> createMetaEntries(final MasterServices masterSer
226231
return createMetaEntriesSuccesses;
227232
}
228233

234+
private static void createRegionDirectories(final MasterServices masterServices,
235+
final List<RegionInfo> regions) {
236+
if (regions.isEmpty()) {
237+
return;
238+
}
239+
final MasterFileSystem mfs = masterServices.getMasterFileSystem();
240+
final Path rootDir = mfs.getRootDir();
241+
for (RegionInfo regionInfo : regions) {
242+
if (regionInfo.getReplicaId() == RegionInfo.DEFAULT_REPLICA_ID) {
243+
try {
244+
Path tableDir = CommonFSUtils.getTableDir(rootDir, regionInfo.getTable());
245+
HRegionFileSystem.createRegionOnFileSystem(masterServices.getConfiguration(),
246+
mfs.getFileSystem(), tableDir, regionInfo);
247+
} catch (IOException e) {
248+
LOG.warn("Failed to create region directory for {}: {}",
249+
regionInfo.getRegionNameAsString(), e.getMessage(), e);
250+
}
251+
}
252+
}
253+
}
254+
229255
/**
230256
* Fix overlaps noted in CJ consistency report.
231257
*/

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -866,7 +866,10 @@ private static void writeRegionInfoFileContent(final Configuration conf, final F
866866
// First check to get the permissions
867867
FsPermission perms = CommonFSUtils.getFilePermissions(fs, conf, HConstants.DATA_FILE_UMASK_KEY);
868868
// Write the RegionInfo file content
869-
try (FSDataOutputStream out = FSUtils.create(conf, fs, regionInfoFile, perms, null)) {
869+
// HBASE-29662: Fail .regioninfo file creation, if the region directory doesn't exist,
870+
// avoiding silent masking of missing region directories during region initialization.
871+
// The region directory should already exist when this method is called.
872+
try (FSDataOutputStream out = FSUtils.create(conf, fs, regionInfoFile, perms, null, false)) {
870873
out.write(content);
871874
}
872875
}
@@ -950,6 +953,14 @@ private void writeRegionInfoOnFilesystem(final byte[] regionInfoContent, final b
950953
CommonFSUtils.delete(fs, tmpPath, true);
951954
}
952955

956+
// Check parent (region) directory exists first to maintain HBASE-29662 protection
957+
if (!fs.exists(getRegionDir())) {
958+
throw new IOException("Region directory does not exist: " + getRegionDir());
959+
}
960+
if (!fs.exists(getTempDir())) {
961+
fs.mkdirs(getTempDir());
962+
}
963+
953964
// Write HRI to a file in case we need to recover hbase:meta
954965
writeRegionInfoFileContent(conf, fs, tmpPath, regionInfoContent);
955966

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

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,32 @@ public static boolean deleteRegionDir(final Configuration conf, final RegionInfo
196196
*/
197197
public static FSDataOutputStream create(Configuration conf, FileSystem fs, Path path,
198198
FsPermission perm, InetSocketAddress[] favoredNodes) throws IOException {
199+
return create(conf, fs, path, perm, favoredNodes, true);
200+
}
201+
202+
/**
203+
* Create the specified file on the filesystem. By default, this will:
204+
* <ol>
205+
* <li>overwrite the file if it exists</li>
206+
* <li>apply the umask in the configuration (if it is enabled)</li>
207+
* <li>use the fs configured buffer size (or 4096 if not set)</li>
208+
* <li>use the configured column family replication or default replication if
209+
* {@link ColumnFamilyDescriptorBuilder#DEFAULT_DFS_REPLICATION}</li>
210+
* <li>use the default block size</li>
211+
* <li>not track progress</li>
212+
* </ol>
213+
* @param conf configurations
214+
* @param fs {@link FileSystem} on which to write the file
215+
* @param path {@link Path} to the file to write
216+
* @param perm permissions
217+
* @param favoredNodes favored data nodes
218+
* @param isRecursiveCreate recursively create parent directories
219+
* @return output stream to the created file
220+
* @throws IOException if the file cannot be created
221+
*/
222+
public static FSDataOutputStream create(Configuration conf, FileSystem fs, Path path,
223+
FsPermission perm, InetSocketAddress[] favoredNodes, boolean isRecursiveCreate)
224+
throws IOException {
199225
if (fs instanceof HFileSystem) {
200226
FileSystem backingFs = ((HFileSystem) fs).getBackingFs();
201227
if (backingFs instanceof DistributedFileSystem) {
@@ -214,7 +240,7 @@ public static FSDataOutputStream create(Configuration conf, FileSystem fs, Path
214240
}
215241

216242
}
217-
return CommonFSUtils.create(fs, path, perm, true);
243+
return CommonFSUtils.create(fs, path, perm, true, isRecursiveCreate);
218244
}
219245

220246
/**

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@
100100
import org.apache.hadoop.hbase.logging.Log4jUtils;
101101
import org.apache.hadoop.hbase.mapreduce.MapreduceTestingShim;
102102
import org.apache.hadoop.hbase.master.HMaster;
103+
import org.apache.hadoop.hbase.master.MasterFileSystem;
103104
import org.apache.hadoop.hbase.master.RegionState;
104105
import org.apache.hadoop.hbase.master.ServerManager;
105106
import org.apache.hadoop.hbase.master.assignment.AssignmentManager;
@@ -110,6 +111,7 @@
110111
import org.apache.hadoop.hbase.regionserver.BloomType;
111112
import org.apache.hadoop.hbase.regionserver.ChunkCreator;
112113
import org.apache.hadoop.hbase.regionserver.HRegion;
114+
import org.apache.hadoop.hbase.regionserver.HRegionFileSystem;
113115
import org.apache.hadoop.hbase.regionserver.HRegionServer;
114116
import org.apache.hadoop.hbase.regionserver.HStore;
115117
import org.apache.hadoop.hbase.regionserver.InternalScanner;
@@ -4277,4 +4279,22 @@ public static void await(final long sleepMillis, final BooleanSupplier condition
42774279
throw e;
42784280
}
42794281
}
4282+
4283+
public void createRegionDir(RegionInfo hri) throws IOException {
4284+
Path rootDir = getDataTestDir();
4285+
Path tableDir = CommonFSUtils.getTableDir(rootDir, hri.getTable());
4286+
Path regionDir = new Path(tableDir, hri.getEncodedName());
4287+
FileSystem fs = getTestFileSystem();
4288+
if (!fs.exists(regionDir)) {
4289+
fs.mkdirs(regionDir);
4290+
}
4291+
}
4292+
4293+
public void createRegionDir(RegionInfo regionInfo, MasterFileSystem masterFileSystem)
4294+
throws IOException {
4295+
Path tableDir =
4296+
CommonFSUtils.getTableDir(CommonFSUtils.getRootDir(conf), regionInfo.getTable());
4297+
HRegionFileSystem.createRegionOnFileSystem(conf, masterFileSystem.getFileSystem(), tableDir,
4298+
regionInfo);
4299+
}
42804300
}

‎hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoreRegionCoprocessor.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,14 @@ public void before() throws IOException {
7676
this.rss = new MockRegionServerServices(HTU.getConfiguration());
7777
ChunkCreator.initialize(MemStoreLAB.CHUNK_SIZE_DEFAULT, false, 0, 0, 0, null,
7878
MemStoreLAB.INDEX_CHUNK_SIZE_PERCENTAGE_DEFAULT);
79+
HTU.createRegionDir(ri);
7980
this.region = HRegion.openHRegion(ri, td, null, HTU.getConfiguration(), this.rss, null);
8081
}
8182

8283
@After
8384
public void after() throws IOException {
8485
this.region.close();
86+
HTU.cleanupTestDir();
8587
}
8688

8789
/**

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ private static RegionInfo makeOverlap(MasterServices services, RegionInfo a, Reg
170170
throws IOException {
171171
RegionInfo overlapRegion = RegionInfoBuilder.newBuilder(a.getTable())
172172
.setStartKey(a.getStartKey()).setEndKey(b.getEndKey()).build();
173+
TEST_UTIL.createRegionDir(overlapRegion, services.getMasterFileSystem());
173174
MetaTableAccessor.putsToMetaTable(services.getConnection(),
174175
Collections.singletonList(MetaTableAccessor.makePutFromRegionInfo(overlapRegion,
175176
EnvironmentEdgeManager.currentTime())));

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,9 +178,11 @@ private HRegion initHRegion(TableDescriptor htd, RegionInfo info) throws IOExcep
178178
CommonFSUtils.setRootDir(walConf, tableDir);
179179
final WALFactory wals = new WALFactory(walConf, "log_" + info.getEncodedName());
180180
HRegion region = new HRegion(fs, wals.getWAL(info), conf, htd, null);
181-
181+
Path regionDir = new Path(tableDir, info.getEncodedName());
182+
if (!fs.getFileSystem().exists(regionDir)) {
183+
fs.getFileSystem().mkdirs(regionDir);
184+
}
182185
region.initialize();
183-
184186
return region;
185187
}
186188

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,7 @@ private HRegion initHRegion(TableDescriptor htd, RegionInfo info) throws IOExcep
194194
.rename(eq(new Path(storeDir, ERROR_FILE)), any());
195195

196196
HRegionFileSystem fs = new HRegionFileSystem(conf, errFS, tableDir, info);
197+
fs.createRegionOnFileSystem(conf, fs.getFileSystem(), tableDir, info);
197198
final Configuration walConf = new Configuration(conf);
198199
CommonFSUtils.setRootDir(walConf, tableDir);
199200
final WALFactory wals = new WALFactory(walConf, "log_" + info.getEncodedName());

0 commit comments

Comments
 (0)