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

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

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/CatalogJanitor.java

@@ -219,6 +219,8 @@ boolean cleanMergeRegion(final HRegionInfo mergedRegion,
           + " from fs because merged region no longer holds references");
       HFileArchiver.archiveRegion(this.services.getConfiguration(), fs, regionA);
       HFileArchiver.archiveRegion(this.services.getConfiguration(), fs, regionB);
+      cleanUpWalDirs(this.services.getMasterFileSystem(), regionA);
+      cleanUpWalDirs(this.services.getMasterFileSystem(), regionB);
       MetaTableAccessor.deleteMergeQualifiers(services.getConnection(), mergedRegion);
       services.getAssignmentManager().getRegionStates().deleteRegion(regionA);
       services.getAssignmentManager().getRegionStates().deleteRegion(regionB);
@@ -365,6 +367,7 @@ boolean cleanParent(final HRegionInfo parent, Result rowContent)
       FileSystem fs = this.services.getMasterFileSystem().getFileSystem();
       if (LOG.isTraceEnabled()) LOG.trace("Archiving parent region: " + parent);
       HFileArchiver.archiveRegion(this.services.getConfiguration(), fs, parent);
+      cleanUpWalDirs(this.services.getMasterFileSystem(), parent);
       MetaTableAccessor.deleteRegion(this.connection, parent);
       if (services.getAssignmentManager().getRegionStates() != null)
         services.getAssignmentManager().getRegionStates().deleteRegion(parent);
@@ -470,4 +473,25 @@ public boolean cleanMergeQualifier(final HRegionInfo region)
     return cleanMergeRegion(region, mergeRegions.getFirst(),
         mergeRegions.getSecond());
   }
+
+  private static void cleanUpWalDirs(MasterFileSystem mfs, HRegionInfo region)
+      throws IOException {
+    // Clean up WAL dirs
+    FileSystem walFs = mfs.getWALFileSystem();
+    // Clean up the directories on WAL filesystem also
+    Path regionWALDir = FSUtils.getWALRegionDir(mfs.getConfiguration(), region);
+    if (walFs.exists(regionWALDir)) {
+      if (!walFs.delete(regionWALDir, true)) {
+        LOG.debug("Unable to delete " + regionWALDir);
+      }
+    }
+    // Clean up any misplaced files from HBASE-20734
+    Path wrongRegionWALDir = FSUtils.getWrongWALRegionDir(mfs.getConfiguration(),
+      region.getTable(), region.getEncodedName());
+    if (walFs.exists(wrongRegionWALDir)) {
+      if (!walFs.delete(wrongRegionWALDir, true)) {
+        LOG.debug("Unable to delete " + wrongRegionWALDir);
+      }
+    }
+  }
 }

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

@@ -214,7 +214,7 @@ public boolean checkFileSystem() {
     return this.walFsOk;
   }
 
-  protected FileSystem getWALFileSystem() {
+  public FileSystem getWALFileSystem() {
     return this.walFs;
   }
 
@@ -691,6 +691,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

@@ -43,7 +43,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;
@@ -323,7 +322,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!");
     }
@@ -332,6 +331,13 @@ protected static void deleteFromFs(final MasterProcedureEnv env,
     if (!fs.delete(tempTableDir, true) && fs.exists(tempTableDir)) {
       throw new IOException("Couldn't delete " + tempTableDir);
     }
+
+    // Delete the directory on wal filesystem
+    FileSystem walFs = mfs.getWALFileSystem();
+    Path tableWALDir = FSUtils.getWALTableDir(env.getMasterConfiguration(), tableName);
+    if (walFs.exists(tableWALDir) && !walFs.delete(tableWALDir, true)) {
+      throw new IOException("Couldn't delete table dir on wal filesystem " + tableWALDir);
+    }
   }
 
   /**

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

@@ -21,8 +21,10 @@
 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;
@@ -4173,7 +4175,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);
       }
     }
@@ -4286,62 +4288,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,
@@ -7206,34 +7222,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

@@ -640,19 +640,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

@@ -351,7 +351,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

@@ -648,8 +648,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);