[GitHub] [hbase] reborn-go commented on a change in pull request #330: HBASE-22617 Recovered WAL directories not getting cleaned up

[GitBox <gi...@apache.org>]

reborn-go commented on a change in pull request #330:
URL: https://github.com/apache/hbase/pull/330#discussion_r453162136



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
##########
@@ -4629,63 +4629,65 @@ protected long replayRecoveredEditsIfAny(Map<byte[], Long> maxSeqIdInStores,
         minSeqIdForTheRegion = maxSeqIdInStore;
       }
     }
-    long seqid = minSeqIdForTheRegion;
+    long seqId = minSeqIdForTheRegion;
 
     FileSystem walFS = getWalFileSystem();
     FileSystem rootFS = getFilesystem();
-    Path regionDir = getWALRegionDir();
-    Path defaultRegionDir = getRegionDir(FSUtils.getRootDir(conf), getRegionInfo());
-
+    Path wrongRegionWALDir = FSUtils.getWrongWALRegionDir(conf, getRegionInfo().getTable(),
+      getRegionInfo().getEncodedName());
+    Path regionWALDir = getWALRegionDir();
+    Path regionDir = FSUtils.getRegionDirFromRootDir(FSUtils.getRootDir(conf), getRegionInfo());
+
+    // We made a mistake in HBASE-20734 so we need to do this dirty hack...
+    NavigableSet<Path> filesUnderWrongRegionWALDir =
+      WALSplitUtil.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 =
-          WALSplitUtil.getSplitEditFilesSorted(rootFS, defaultRegionDir);
-      seqid = Math.max(seqid,
-          replayRecoveredEditsForPaths(minSeqIdForTheRegion, rootFS, filesUnderRootDir, reporter,
-              defaultRegionDir));
+    NavigableSet<Path> filesUnderRootDir = Collections.emptyNavigableSet();
+    if (!regionWALDir.equals(regionDir)) {
+      filesUnderRootDir = WALSplitUtil.getSplitEditFilesSorted(rootFS, regionDir);
+      seqId = Math.max(seqId, replayRecoveredEditsForPaths(minSeqIdForTheRegion, rootFS,
+        filesUnderRootDir, reporter, regionDir));
     }
 
-    NavigableSet<Path> files = WALSplitUtil.getSplitEditFilesSorted(walFS, regionDir);
-    seqid = Math.max(seqid, replayRecoveredEditsForPaths(minSeqIdForTheRegion, walFS,
-        files, reporter, regionDir));
+    NavigableSet<Path> files = WALSplitUtil.getSplitEditFilesSorted(walFS, regionWALDir);
+    seqId = Math.max(seqId, replayRecoveredEditsForPaths(minSeqIdForTheRegion, walFS,
+        files, reporter, regionWALDir));
 
-    if (seqid > minSeqIdForTheRegion) {
+    if (seqId > minSeqIdForTheRegion) {
       // Then we added some edits to memory. Flush and cleanup split edit files.
-      internalFlushcache(null, seqid, stores.values(), status, false, FlushLifeCycleTracker.DUMMY);
+      internalFlushcache(null, seqId, stores.values(), status, false, FlushLifeCycleTracker.DUMMY);
     }
-    // Now delete the content of recovered edits.  We're done w/ them.
+    // 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)) {
       // 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 = WALSplitUtil.getRegionDirRecoveredEditsDir(regionDir).getName();
+      String fakeFamilyName = WALSplitUtil.getRegionDirRecoveredEditsDir(regionWALDir).getName();
       Set<HStoreFile> fakeStoreFiles = new HashSet<>(files.size());
-      for (Path file: files) {
-        fakeStoreFiles.add(
-          new HStoreFile(walFS, file, this.conf, null, null, true));
+      for (Path file : files) {
+        fakeStoreFiles.add(new HStoreFile(walFS, file, this.conf, null, null, true));
       }
       getRegionWALFileSystem().removeStoreFiles(fakeFamilyName, fakeStoreFiles);
     } else {
-      if (filesUnderRootDir != null) {
-        for (Path file : filesUnderRootDir) {
-          if (!rootFS.delete(file, false)) {
-            LOG.error("Failed delete of {} from under the root directory.", file);
-          } else {
-            LOG.debug("Deleted recovered.edits under root directory. file=" + file);
-          }
+      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);
         }
       }
-      for (Path file: files) {
-        if (!walFS.delete(file, false)) {
-          LOG.error("Failed delete of " + file);
+      for (Path file : filesUnderRootDir) {

Review comment:
       hi, if the region is read-only mode for HbaseTableSnapshotInputFormat, it try to delete the filesUnderRootDir and no access, it will throw exception and failed




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org