From 21ebbdae06f91d8f7ce04057288cf386d5b561cf Mon Sep 17 00:00:00 2001 From: mbertozzi Date: Tue, 15 Jan 2013 16:51:44 +0000 Subject: [PATCH] HBASE-7537 .regioninfo not created by createHRegion() git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1433514 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop/hbase/regionserver/HRegion.java | 68 ++++++++++++++----- .../hbase/regionserver/TestHRegion.java | 50 +++++++++++++- 2 files changed, 99 insertions(+), 19 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index c0b449a62c3..e2703cdd2d3 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -753,27 +753,49 @@ public class HRegion implements HeapSize { // , Writable{ return this.memstoreSize.getAndAdd(memStoreSize); } - /* - * Write out an info file under the region directory. Useful recovering - * mangled regions. + /** + * Write out an info file under the stored region directory. Useful recovering mangled regions. * @throws IOException */ private void checkRegioninfoOnFilesystem() throws IOException { - Path regioninfoPath = new Path(this.regiondir, REGIONINFO_FILE); - // Compose the content of the file so we can compare to length in filesystem. If not same, - // rewrite it (it may have been written in the old format using Writables instead of pb). The + checkRegioninfoOnFilesystem(this.regiondir); + } + + /** + * Write out an info file under the region directory. Useful recovering mangled regions. + * @param regiondir directory under which to write out the region info + * @throws IOException + */ + private void checkRegioninfoOnFilesystem(Path regiondir) throws IOException { + writeRegioninfoOnFilesystem(regionInfo, regiondir, getFilesystem(), conf); + } + + /** + * Write out an info file under the region directory. Useful recovering mangled regions. If the + * regioninfo already exists on disk and there is information in the file, then we fast exit. + * @param regionInfo information about the region + * @param regiondir directory under which to write out the region info + * @param fs {@link FileSystem} on which to write the region info + * @param conf {@link Configuration} from which to extract specific file locations + * @throws IOException on unexpected error. + */ + public static void writeRegioninfoOnFilesystem(HRegionInfo regionInfo, Path regiondir, + FileSystem fs, Configuration conf) throws IOException { + Path regioninfoPath = new Path(regiondir, REGIONINFO_FILE); + // Compose the content of the file so we can compare to length in filesystem. If not same, + // rewrite it (it may have been written in the old format using Writables instead of pb). The // pb version is much shorter -- we write now w/o the toString version -- so checking length - // only should be sufficient. I don't want to read the file every time to check if it pb + // only should be sufficient. I don't want to read the file every time to check if it pb // serialized. - byte [] content = getDotRegionInfoFileContent(this.getRegionInfo()); - boolean exists = this.fs.exists(regioninfoPath); - FileStatus status = exists? this.fs.getFileStatus(regioninfoPath): null; + byte[] content = getDotRegionInfoFileContent(regionInfo); + boolean exists = fs.exists(regioninfoPath); + FileStatus status = exists ? fs.getFileStatus(regioninfoPath) : null; if (status != null && status.getLen() == content.length) { // Then assume the content good and move on. return; } // Create in tmpdir and then move into place in case we crash after - // create but before close. If we don't successfully close the file, + // create but before close. If we don't successfully close the file, // subsequent region reopens will fail the below because create is // registered in NN. @@ -781,7 +803,7 @@ public class HRegion implements HeapSize { // , Writable{ FsPermission perms = FSUtils.getFilePermissions(fs, conf, HConstants.DATA_FILE_UMASK_KEY); // And then create the file - Path tmpPath = new Path(getTmpDir(), REGIONINFO_FILE); + Path tmpPath = new Path(getTmpDir(regiondir), REGIONINFO_FILE); // If datanode crashes or if the RS goes down just before the close is called while trying to // close the created regioninfo file in the .tmp directory then on next @@ -1227,7 +1249,15 @@ public class HRegion implements HeapSize { // , Writable{ * will have its contents removed when the region is reopened. */ Path getTmpDir() { - return new Path(getRegionDir(), REGION_TEMP_SUBDIR); + return getTmpDir(getRegionDir()); + } + + /** + * Get the temporary directory for the specified region. This directory + * will have its contents removed when the region is reopened. + */ + static Path getTmpDir(Path regionDir) { + return new Path(regionDir, REGION_TEMP_SUBDIR); } void triggerMajorCompaction() { @@ -3924,6 +3954,8 @@ public class HRegion implements HeapSize { // , Writable{ Path regionDir = HRegion.getRegionDir(tableDir, info.getEncodedName()); FileSystem fs = FileSystem.get(conf); fs.mkdirs(regionDir); + // Write HRI to a file in case we need to recover .META. + writeRegioninfoOnFilesystem(info, regionDir, fs, conf); HLog effectiveHLog = hlog; if (hlog == null && !ignoreHLog) { effectiveHLog = HLogFactory.createHLog(fs, regionDir, @@ -4004,15 +4036,15 @@ public class HRegion implements HeapSize { // , Writable{ return r.openHRegion(reporter); } - public static HRegion openHRegion(Path tableDir, final HRegionInfo info, + public static HRegion openHRegion(Path rootDir, final HRegionInfo info, final HTableDescriptor htd, final HLog wal, final Configuration conf) throws IOException { - return openHRegion(tableDir, info, htd, wal, conf, null, null); + return openHRegion(rootDir, info, htd, wal, conf, null, null); } /** * Open a Region. - * @param tableDir Table directory + * @param rootDir Root directory for HBase instance * @param info Info for region to be opened. * @param wal HLog for region to use. This method will call * HLog#setSequenceNumber(long) passing the result of the call to @@ -4024,7 +4056,7 @@ public class HRegion implements HeapSize { // , Writable{ * * @throws IOException */ - public static HRegion openHRegion(final Path tableDir, final HRegionInfo info, + public static HRegion openHRegion(final Path rootDir, final HRegionInfo info, final HTableDescriptor htd, final HLog wal, final Configuration conf, final RegionServerServices rsServices, final CancelableProgressable reporter) @@ -4034,7 +4066,7 @@ public class HRegion implements HeapSize { // , Writable{ if (LOG.isDebugEnabled()) { LOG.debug("Opening region: " + info); } - Path dir = HTableDescriptor.getTableDir(tableDir, info.getTableName()); + Path dir = HTableDescriptor.getTableDir(rootDir, info.getTableName()); HRegion r = HRegion.newHRegion(dir, wal, FileSystem.get(conf), conf, info, htd, rsServices); return r.openHRegion(reporter); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java index 317b9396df3..ab9a2996ed9 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java @@ -3506,7 +3506,55 @@ public class TestHRegion extends HBaseTestCase { HRegion.closeHRegion(region); } } - + + /** + * Verifies that the .regioninfo file is written on region creation + * and that is recreated if missing during region opening. + */ + public void testRegionInfoFileCreation() throws IOException { + Path rootDir = new Path(DIR + "testRegionInfoFileCreation"); + Configuration conf = HBaseConfiguration.create(this.conf); + + HTableDescriptor htd = new HTableDescriptor("testtb"); + htd.addFamily(new HColumnDescriptor("cf")); + + HRegionInfo hri = new HRegionInfo(htd.getName()); + + // Create a region and skip the initialization (like CreateTableHandler) + HRegion region = HRegion.createHRegion(hri, rootDir, conf, htd, null, false, true); + Path regionDir = region.getRegionDir(); + FileSystem fs = region.getFilesystem(); + HRegion.closeHRegion(region); + + Path regionInfoFile = new Path(regionDir, HRegion.REGIONINFO_FILE); + + // Verify that the .regioninfo file is present + assertTrue(HRegion.REGIONINFO_FILE + " should be present in the region dir", + fs.exists(regionInfoFile)); + + // Try to open the region + region = HRegion.openHRegion(rootDir, hri, htd, null, conf); + assertEquals(regionDir, region.getRegionDir()); + HRegion.closeHRegion(region); + + // Verify that the .regioninfo file is still there + assertTrue(HRegion.REGIONINFO_FILE + " should be present in the region dir", + fs.exists(regionInfoFile)); + + // Remove the .regioninfo file and verify is recreated on region open + fs.delete(regionInfoFile); + assertFalse(HRegion.REGIONINFO_FILE + " should be removed from the region dir", + fs.exists(regionInfoFile)); + + region = HRegion.openHRegion(rootDir, hri, htd, null, conf); + assertEquals(regionDir, region.getRegionDir()); + HRegion.closeHRegion(region); + + // Verify that the .regioninfo file is still there + assertTrue(HRegion.REGIONINFO_FILE + " should be present in the region dir", + fs.exists(new Path(regionDir, HRegion.REGIONINFO_FILE))); + } + /** * TestCase for increment *