HBASE-22627 Port HBASE-22617 (Recovered WAL directories not getting cleaned up) to branch-1 (#339)

HBASE-22617 Recovered WAL directories not getting cleaned up (Duo Zhang)

Signed-off-by: Zach York <zyork@apache.org>

Author: apurtell

hbase-server/src/main/java/org/apache/hadoop/hbase/backup/HFileArchiver.java

@@ -32,10 +32,8 @@
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.PathFilter;
-import org.apache.hadoop.hbase.HBaseInterfaceAudience;
 import org.apache.hadoop.hbase.HRegionInfo;
 import org.apache.hadoop.hbase.classification.InterfaceAudience;
-import org.apache.hadoop.hbase.regionserver.HRegion;
 import org.apache.hadoop.hbase.regionserver.StoreFile;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
@@ -85,7 +83,7 @@ public static void archiveRegion(Configuration conf, FileSystem fs, HRegionInfo
       throws IOException {
     Path rootDir = FSUtils.getRootDir(conf);
     archiveRegion(fs, rootDir, FSUtils.getTableDir(rootDir, info.getTable()),
-      HRegion.getRegionDir(rootDir, info));
+      FSUtils.getRegionDirFromRootDir(rootDir, info));
   }
 
   /**

hbase-server/src/main/java/org/apache/hadoop/hbase/io/HFileLink.java

@@ -31,7 +31,6 @@
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HRegionInfo;
-import org.apache.hadoop.hbase.regionserver.HRegion;
 import org.apache.hadoop.hbase.regionserver.StoreFileInfo;
 import org.apache.hadoop.hbase.util.FSUtils;
 import org.apache.hadoop.hbase.util.HFileArchiveUtil;
@@ -486,7 +485,7 @@ public static Path getHFileFromBackReference(final Path rootDir, final Path link
     String linkName = createHFileLinkName(FSUtils.getTableName(tablePath),
             regionPath.getName(), hfileName);
     Path linkTableDir = FSUtils.getTableDir(rootDir, linkTableName);
-    Path regionDir = HRegion.getRegionDir(linkTableDir, linkRegionName);
+    Path regionDir = new Path(linkTableDir, linkRegionName);
     return new Path(new Path(regionDir, familyPath.getName()), linkName);
   }
 

hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java

@@ -212,7 +212,7 @@ public boolean checkFileSystem() {
     return this.walFsOk;
   }
 
-  protected FileSystem getWALFileSystem() {
+  public FileSystem getWALFileSystem() {
     return this.walFs;
   }
 
@@ -676,6 +676,4 @@ public void archiveMetaLog(final ServerName serverName) {
       LOG.warn("Failed archiving meta log for server " + serverName, ie);
     }
   }
-
-
 }

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

@@ -44,7 +44,6 @@
 import org.apache.hadoop.hbase.client.Scan;
 import org.apache.hadoop.hbase.client.Table;
 import org.apache.hadoop.hbase.exceptions.HBaseException;
-import org.apache.hadoop.hbase.regionserver.HRegion;
 import org.apache.hadoop.hbase.master.AssignmentManager;
 import org.apache.hadoop.hbase.master.MasterCoprocessorHost;
 import org.apache.hadoop.hbase.master.MasterFileSystem;
@@ -336,7 +335,7 @@ protected static void deleteFromFs(final MasterProcedureEnv env,
       for (HRegionInfo hri : regions) {
         LOG.debug("Archiving region " + hri.getRegionNameAsString() + " from FS");
         HFileArchiver.archiveRegion(fs, mfs.getRootDir(),
-            tempTableDir, HRegion.getRegionDir(tempTableDir, hri.getEncodedName()));
+            tempTableDir, new Path(tempTableDir, hri.getEncodedName()));
       }
       LOG.debug("Table '" + tableName + "' archived!");
     }

hbase-server/src/main/java/org/apache/hadoop/hbase/migration/NamespaceUpgrade.java

@@ -319,8 +319,7 @@ public void migrateMeta() throws IOException {
     }
 
     // Since meta table name has changed rename meta region dir from it's old encoding to new one
-    Path oldMetaRegionDir = HRegion.getRegionDir(rootDir,
-      new Path(newMetaDir, "1028785192").toString());
+    Path oldMetaRegionDir = new Path(rootDir, new Path(newMetaDir, "1028785192").toString());
     if (fs.exists(oldMetaRegionDir)) {
       LOG.info("Migrating meta region " + oldMetaRegionDir + " to " + newMetaRegionDir);
       if (!fs.rename(oldMetaRegionDir, newMetaRegionDir)) {

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

@@ -18,6 +18,22 @@
  */
 package org.apache.hadoop.hbase.regionserver;
 
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
+import com.google.common.io.Closeables;
+import com.google.protobuf.ByteString;
+import com.google.protobuf.Descriptors;
+import com.google.protobuf.Message;
+import com.google.protobuf.RpcCallback;
+import com.google.protobuf.RpcController;
+import com.google.protobuf.Service;
+import com.google.protobuf.TextFormat;
+
 import java.io.EOFException;
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -187,20 +203,6 @@
 import org.apache.htrace.Trace;
 import org.apache.htrace.TraceScope;
 
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Optional;
-import com.google.common.base.Preconditions;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
-import com.google.common.io.Closeables;
-import com.google.protobuf.ByteString;
-import com.google.protobuf.Descriptors;
-import com.google.protobuf.Message;
-import com.google.protobuf.RpcCallback;
-import com.google.protobuf.RpcController;
-import com.google.protobuf.Service;
-import com.google.protobuf.TextFormat;
-
 @InterfaceAudience.Private
 public class HRegion implements HeapSize, PropagatingConfigurationObserver, Region {
   private static final Log LOG = LogFactory.getLog(HRegion.class);
@@ -4033,7 +4035,7 @@ private void removeNonExistentColumnFamilyForReplay(
     if (nonExistentList != null) {
       for (byte[] family : nonExistentList) {
         // Perhaps schema was changed between crash and replay
-        LOG.info("No family for " + Bytes.toString(family) + " omit from reply.");
+        LOG.info("No family for " + Bytes.toString(family) + " omit from replay.");
         familyMap.remove(family);
       }
     }
@@ -4146,62 +4148,76 @@ protected long replayRecoveredEditsIfAny(Map<byte[], Long> maxSeqIdInStores,
         minSeqIdForTheRegion = maxSeqIdInStore;
       }
     }
-    long seqid = minSeqIdForTheRegion;
+    long seqId = minSeqIdForTheRegion;
 
     FileSystem walFS = getWalFileSystem();
-    Path regionDir = getWALRegionDir();
     FileSystem rootFS = getFilesystem();
-    Path defaultRegionDir = getRegionDir(FSUtils.getRootDir(conf), getRegionInfo());
+    Path regionDir = FSUtils.getRegionDirFromRootDir(FSUtils.getRootDir(conf), getRegionInfo());
+    Path regionWALDir = getWALRegionDir();
+    Path wrongRegionWALDir = FSUtils.getWrongWALRegionDir(conf, getRegionInfo().getTable(),
+      getRegionInfo().getEncodedName());
 
+    // We made a mistake in HBASE-20734 so we need to do this dirty hack...
+    NavigableSet<Path> filesUnderWrongRegionWALDir =
+      WALSplitter.getSplitEditFilesSorted(walFS, wrongRegionWALDir);
+    seqId = Math.max(seqId, replayRecoveredEditsForPaths(minSeqIdForTheRegion, walFS,
+      filesUnderWrongRegionWALDir, reporter, regionDir));
     // This is to ensure backwards compatability with HBASE-20723 where recovered edits can appear
     // under the root dir even if walDir is set.
-    NavigableSet<Path> filesUnderRootDir = null;
-    if (!regionDir.equals(defaultRegionDir)) {
-      filesUnderRootDir =
-          WALSplitter.getSplitEditFilesSorted(rootFS, defaultRegionDir);
-      seqid = Math.max(seqid,
-          replayRecoveredEditsForPaths(minSeqIdForTheRegion, rootFS, filesUnderRootDir, reporter,
-              defaultRegionDir));
-    }
-    NavigableSet<Path> files = WALSplitter.getSplitEditFilesSorted(walFS, regionDir);
-    seqid = Math.max(seqid, replayRecoveredEditsForPaths(minSeqIdForTheRegion, walFS,
-        files, reporter, regionDir));
-
-    if (seqid > minSeqIdForTheRegion) {
+    NavigableSet<Path> filesUnderRootDir = Sets.newTreeSet();
+    if (!regionWALDir.equals(regionDir)) {
+      filesUnderRootDir = WALSplitter.getSplitEditFilesSorted(rootFS, regionDir);
+      seqId = Math.max(seqId, replayRecoveredEditsForPaths(minSeqIdForTheRegion, rootFS,
+        filesUnderRootDir, reporter, regionDir));
+    }
+    NavigableSet<Path> files = WALSplitter.getSplitEditFilesSorted(walFS, regionWALDir);
+    seqId = Math.max(seqId, replayRecoveredEditsForPaths(minSeqIdForTheRegion, walFS,
+        files, reporter, regionWALDir));
+    if (seqId > minSeqIdForTheRegion) {
       // Then we added some edits to memory. Flush and cleanup split edit files.
-      internalFlushcache(null, seqid, stores.values(), status, false);
+      internalFlushcache(null, seqId, stores.values(), status, false);
     }
-    // Now delete the content of recovered edits.  We're done w/ them.
-    if (files.size() > 0 && this.conf.getBoolean("hbase.region.archive.recovered.edits", false)) {
+    // Now delete the content of recovered edits. We're done w/ them.
+    if (conf.getBoolean("hbase.region.archive.recovered.edits", false)) {
       // For debugging data loss issues!
       // If this flag is set, make use of the hfile archiving by making recovered.edits a fake
       // column family. Have to fake out file type too by casting our recovered.edits as storefiles
-      String fakeFamilyName = WALSplitter.getRegionDirRecoveredEditsDir(regionDir).getName();
-      Set<StoreFile> fakeStoreFiles = new HashSet<>(files.size());
-      for (Path file: files) {
-        fakeStoreFiles.add(
-            new StoreFile(walFS, file, this.conf, null, null));
+      String fakeFamilyName = WALSplitter.getRegionDirRecoveredEditsDir(regionWALDir).getName();
+      Set<StoreFile> fakeStoreFiles = new HashSet<>();
+      for (Path file: Iterables.concat(files, filesUnderWrongRegionWALDir)) {
+        fakeStoreFiles.add(new StoreFile(walFS, file, conf, null, null));
+      }
+      for (Path file: filesUnderRootDir) {
+        fakeStoreFiles.add(new StoreFile(rootFS, file, conf, null, null));
       }
       getRegionWALFileSystem().removeStoreFiles(fakeFamilyName, fakeStoreFiles);
     } else {
-      if (filesUnderRootDir != null) {
-        for (Path file : filesUnderRootDir) {
-          if (!rootFS.delete(file, false)) {
-            LOG.error("Failed delete of {} under root directory." + file);
-          } else {
-            LOG.debug("Deleted recovered.edits root directory file=" + file);
-          }
+      for (Path file : filesUnderRootDir) {
+        if (!rootFS.delete(file, false)) {
+          LOG.error("Failed delete of " + file + " from under the root directory");
+        } else {
+          LOG.debug("Deleted recovered.edits under root directory, file=" + file);
         }
       }
-      for (Path file: files) {
+      for (Path file : Iterables.concat(files, filesUnderWrongRegionWALDir)) {
         if (!walFS.delete(file, false)) {
           LOG.error("Failed delete of " + file);
         } else {
           LOG.debug("Deleted recovered.edits file=" + file);
         }
       }
     }
-    return seqid;
+
+    // We have replayed all the recovered edits. Let's delete the wrong directories introduced
+    // in HBASE-20734, see HBASE-22617 for more details.
+    FileSystem walFs = getWalFileSystem();
+    if (walFs.exists(wrongRegionWALDir)) {
+      if (!walFs.delete(wrongRegionWALDir, true)) {
+        LOG.warn("Unable to delete " + wrongRegionWALDir);
+      }
+    }
+
+    return seqId;
   }
 
   private long replayRecoveredEditsForPaths(long minSeqIdForTheRegion, FileSystem fs,
@@ -7023,34 +7039,6 @@ public static void addRegionToMETA(final HRegion meta, final HRegion r) throws I
     meta.put(row, HConstants.CATALOG_FAMILY, cells);
   }
 
-  /**
-   * Computes the Path of the HRegion
-   *
-   * @param tabledir qualified path for table
-   * @param name ENCODED region name
-   * @return Path of HRegion directory
-   * @deprecated For tests only; to be removed.
-   */
-  @Deprecated
-  public static Path getRegionDir(final Path tabledir, final String name) {
-    return new Path(tabledir, name);
-  }
-
-  /**
-   * Computes the Path of the HRegion
-   *
-   * @param rootdir qualified path of HBase root directory
-   * @param info HRegionInfo for the region
-   * @return qualified path of region directory
-   * @deprecated For tests only; to be removed.
-   */
-  @Deprecated
-  @VisibleForTesting
-  public static Path getRegionDir(final Path rootdir, final HRegionInfo info) {
-    return new Path(
-      FSUtils.getTableDir(rootdir, info.getTable()), info.getEncodedName());
-  }
-
   /**
    * Determines if the specified row is within the row range specified by the
    * specified HRegionInfo

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

@@ -557,19 +557,26 @@ Path commitDaughterRegion(final HRegionInfo regionInfo)
   /**
    * Create the region splits directory.
    */
-  void createSplitsDir() throws IOException {
+  void createSplitsDir(HRegionInfo daughterA, HRegionInfo daughterB) throws IOException {
     Path splitdir = getSplitsDir();
     if (fs.exists(splitdir)) {
       LOG.info("The " + splitdir + " directory exists.  Hence deleting it to recreate it");
       if (!deleteDir(splitdir)) {
-        throw new IOException("Failed deletion of " + splitdir
-            + " before creating them again.");
+        throw new IOException("Failed deletion of " + splitdir + " before creating them again.");
       }
     }
     // splitDir doesn't exists now. No need to do an exists() call for it.
     if (!createDir(splitdir)) {
       throw new IOException("Failed create of " + splitdir);
     }
+    Path daughterATmpDir = getSplitsDir(daughterA);
+    if (!createDir(daughterATmpDir)) {
+      throw new IOException("Failed create of " + daughterATmpDir);
+    }
+    Path daughterBTmpDir = getSplitsDir(daughterB);
+    if (!createDir(daughterBTmpDir)) {
+      throw new IOException("Failed create of " + daughterBTmpDir);
+    }
   }
 
   /**

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

@@ -383,7 +383,7 @@ public PairOfSameType<Region> stepsBeforePONR(final Server server,
             hri_b, std);
     }
 
-    this.parent.getRegionFileSystem().createSplitsDir();
+    this.parent.getRegionFileSystem().createSplitsDir(hri_a, hri_b);
 
     transition(SplitTransactionPhase.CREATE_SPLIT_DIR);
 

hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/RestoreSnapshotHelper.java

@@ -642,8 +642,9 @@ private void restoreReferenceFile(final Path familyDir, final HRegionInfo region
       if (linkPath != null) {
         in = HFileLink.buildFromHFileLinkPattern(conf, linkPath).open(fs);
       } else {
-        linkPath = new Path(new Path(HRegion.getRegionDir(snapshotManifest.getSnapshotDir(),
-                        regionInfo.getEncodedName()), familyDir.getName()), hfileName);
+        linkPath = new Path(new Path(new Path(snapshotManifest.getSnapshotDir(),
+          regionInfo.getEncodedName()),
+          familyDir.getName()), hfileName);
         in = fs.open(linkPath);
       }
       OutputStream out = fs.create(outPath);