apurtell 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_r296840532
##########
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:
Small code smell. Now that we have three lists, why not create one
List<Path>, add to the one lists the files we find in any of the three
locations we must check, then process the one list all at once. The only
difference here is the log messages are slightly different but operators and us
won't care if "root" or "wrong". The error or debug log lines all contain the
file path, which is enough, I think.
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
