svn commit: r1433514 - in /hbase/trunk/hbase-server/src: main/java/org/apache/hadoop/hbase/regionserver/HRegion.java test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java

[mb...@apache.org]

Author: mbertozzi
Date: Tue Jan 15 16:51:44 2013
New Revision: 1433514

URL: http://svn.apache.org/viewvc?rev=1433514&view=rev
Log:
HBASE-7537 .regioninfo not created by createHRegion()

Modified:
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
    hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java?rev=1433514&r1=1433513&r2=1433514&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Tue Jan 15 16:51:44 2013
@@ -753,27 +753,49 @@ public class HRegion implements HeapSize
     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
     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
    * 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
     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
     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
    *
    * @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
     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);
   }

Modified: hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java?rev=1433514&r1=1433513&r2=1433514&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java (original)
+++ hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java Tue Jan 15 16:51:44 2013
@@ -3506,7 +3506,55 @@ public class TestHRegion extends HBaseTe
       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
    *