Apache9 commented on a change in pull request #330: HBASE-22617 Recovered WAL 
directories not getting cleaned up
URL: https://github.com/apache/hbase/pull/330#discussion_r296966724
 
 

 ##########
 File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
 ##########
 @@ -4629,63 +4626,72 @@ 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 : files) {
+        if (!walFS.delete(file, false)) {
+          LOG.error("Failed delete of {}", file);
+        } else {
+          LOG.debug("Deleted recovered.edits file={}", file);
+        }
+      }
+      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: files) {
+      for (Path file : filesUnderWrongRegionWALDir) {
 
 Review comment:
   They are on different file systems so at least we need two loops...

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to