Index: hbase-server/src/main/java/org/apache/hadoop/hbase/backup/example/LongTermArchivingHFileCleaner.java =================================================================== --- hbase-server/src/main/java/org/apache/hadoop/hbase/backup/example/LongTermArchivingHFileCleaner.java (revision 1505831) +++ hbase-server/src/main/java/org/apache/hadoop/hbase/backup/example/LongTermArchivingHFileCleaner.java (working copy) @@ -48,11 +48,12 @@ private FileSystem fs; @Override - public boolean isFileDeletable(Path file) { + public boolean isFileDeletable(FileStatus fStat) { try { // if its a directory, then it can be deleted - if (!fs.isFile(file)) return true; - + if (fStat.isDir()) return true; + + Path file = fStat.getPath(); // check to see if FileStatus[] deleteStatus = FSUtils.listStatus(this.fs, file, null); // if the file doesn't exist, then it can be deleted (but should never @@ -69,7 +70,7 @@ LOG.debug("Archiver says to [" + (ret ? "delete" : "keep") + "] files for table:" + tableName); return ret; } catch (IOException e) { - LOG.error("Failed to lookup status of:" + file + ", keeping it just incase.", e); + LOG.error("Failed to lookup status of:" + fStat.getPath() + ", keeping it just incase.", e); return false; } } Index: hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/BaseLogCleanerDelegate.java =================================================================== --- hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/BaseLogCleanerDelegate.java (revision 1505831) +++ hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/BaseLogCleanerDelegate.java (working copy) @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.master.cleaner; import org.apache.hadoop.classification.InterfaceAudience; +import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.BaseConfigurable; @@ -40,19 +41,19 @@ public abstract class BaseLogCleanerDelegate extends BaseConfigurable implements FileCleanerDelegate { @Override - public boolean isFileDeletable(Path file) { - return isLogDeletable(file); + public boolean isFileDeletable(FileStatus fStat) { + return isLogDeletable(fStat); } /** * Should the master delete the log or keep it? *

- * Implementing classes should override {@link #isFileDeletable(Path)} instead. - * @param filePath full path to log. + * Implementing classes should override {@link #isFileDeletable(FileStatus)} instead. + * @param fStat file status of the file * @return true if the log is deletable, false (default) if not */ @Deprecated - public boolean isLogDeletable(Path filePath) { + public boolean isLogDeletable(FileStatus fStat) { return false; } } Index: hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java =================================================================== --- hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java (revision 1505831) +++ hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java (working copy) @@ -122,7 +122,7 @@ for (FileStatus file : files) { try { if (file.isDir()) checkAndDeleteDirectory(file.getPath()); - else checkAndDelete(file.getPath()); + else checkAndDelete(file); } catch (IOException e) { e = RemoteExceptionHandler.checkIOException(e); LOG.warn("Error while cleaning the logs", e); @@ -173,7 +173,7 @@ } } // otherwise we can just check the file - else if (!checkAndDelete(path)) { + else if (!checkAndDelete(child)) { canDeleteThis = false; } } @@ -199,11 +199,12 @@ /** * Run the given file through each of the cleaners to see if it should be deleted, deleting it if * necessary. - * @param filePath path of the file to check (and possibly delete) + * @param fStat path of the file to check (and possibly delete) * @throws IOException if cann't delete a file because of a filesystem issue * @throws IllegalArgumentException if the file is a directory and has children */ - private boolean checkAndDelete(Path filePath) throws IOException, IllegalArgumentException { + private boolean checkAndDelete(FileStatus fStat) throws IOException, IllegalArgumentException { + Path filePath = fStat.getPath(); // first check to see if the path is valid if (!validate(filePath)) { LOG.warn("Found a wrongly formatted file: " + filePath.getName() + " deleting it."); @@ -223,7 +224,7 @@ return false; } - if (!cleaner.isFileDeletable(filePath)) { + if (!cleaner.isFileDeletable(fStat)) { // this file is not deletable, then we are done if (LOG.isTraceEnabled()) { LOG.trace(filePath + " is not deletable according to:" + cleaner); Index: hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/FileCleanerDelegate.java =================================================================== --- hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/FileCleanerDelegate.java (revision 1505831) +++ hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/FileCleanerDelegate.java (working copy) @@ -19,7 +19,7 @@ import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.conf.Configurable; -import org.apache.hadoop.fs.Path; +import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.hbase.Stoppable; /** @@ -32,7 +32,8 @@ /** * Should the master delete the file or keep it? * @param file full path to the file to check + * @param fStat file status of the file to check * @return true if the file is deletable, false if not */ - boolean isFileDeletable(Path file); + boolean isFileDeletable(FileStatus fStat); } Index: hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileLinkCleaner.java =================================================================== --- hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileLinkCleaner.java (revision 1505831) +++ hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileLinkCleaner.java (working copy) @@ -25,6 +25,7 @@ import org.apache.commons.logging.LogFactory; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; @@ -48,9 +49,9 @@ private FileSystem fs = null; @Override - public synchronized boolean isFileDeletable(Path filePath) { + public synchronized boolean isFileDeletable(FileStatus fStat) { if (this.fs == null) return false; - + Path filePath = fStat.getPath(); // HFile Link is always deletable if (HFileLink.isHFileLink(filePath)) return true; Index: hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/TimeToLiveHFileCleaner.java =================================================================== --- hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/TimeToLiveHFileCleaner.java (revision 1505831) +++ hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/TimeToLiveHFileCleaner.java (working copy) @@ -17,15 +17,11 @@ */ package org.apache.hadoop.hbase.master.cleaner; -import java.io.IOException; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileStatus; -import org.apache.hadoop.fs.FileSystem; -import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; /** @@ -41,7 +37,6 @@ public static final long DEFAULT_TTL = 60000 * 5; // Configured time a hfile can be kept after it was moved to the archive private long ttl; - private FileSystem fs; @Override public void setConf(Configuration conf) { @@ -50,45 +45,19 @@ } @Override - public boolean isFileDeletable(Path filePath) { - if (!instantiateFS()) { - return false; - } - long time = 0; + public boolean isFileDeletable(FileStatus fStat) { long currentTime = EnvironmentEdgeManager.currentTimeMillis(); - try { - FileStatus fStat = fs.getFileStatus(filePath); - time = fStat.getModificationTime(); - } catch (IOException e) { - LOG.error("Unable to get modification time of file " + filePath.getName() - + ", not deleting it.", e); - return false; - } + long time = fStat.getModificationTime(); long life = currentTime - time; if (LOG.isTraceEnabled()) { LOG.trace("HFile life:" + life + ", ttl:" + ttl + ", current:" + currentTime + ", from: " + time); } if (life < 0) { - LOG.warn("Found a log (" + filePath + ") newer than current time (" + currentTime + " < " - + time + "), probably a clock skew"); + LOG.warn("Found a hfile (" + fStat.getPath() + ") newer than current time (" + currentTime + + " < " + time + "), probably a clock skew"); return false; } return life > ttl; } - - /** - * setup the filesystem, if it hasn't been already - */ - private synchronized boolean instantiateFS() { - if (this.fs == null) { - try { - this.fs = FileSystem.get(this.getConf()); - } catch (IOException e) { - LOG.error("Couldn't instantiate the file system, not deleting file, just incase"); - return false; - } - } - return true; - } } Index: hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/TimeToLiveLogCleaner.java =================================================================== --- hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/TimeToLiveLogCleaner.java (revision 1505831) +++ hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/TimeToLiveLogCleaner.java (working copy) @@ -17,14 +17,12 @@ */ package org.apache.hadoop.hbase.master.cleaner; -import java.io.IOException; - -import org.apache.hadoop.fs.FileStatus; -import org.apache.hadoop.fs.Path; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.fs.FileStatus; +import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; /** * Log cleaner that uses the timestamp of the hlog to determine if it should @@ -38,21 +36,18 @@ private boolean stopped = false; @Override - public boolean isLogDeletable(Path filePath) { - long time = 0; - long currentTime = System.currentTimeMillis(); - try { - FileStatus fStat = filePath.getFileSystem(this.getConf()).getFileStatus(filePath); - time = fStat.getModificationTime(); - } catch (IOException e) { - LOG.error("Unable to get modification time of file " + filePath.getName() + - ", not deleting it.", e); - return false; + public boolean isLogDeletable(FileStatus fStat) { + long currentTime = EnvironmentEdgeManager.currentTimeMillis(); + long time = fStat.getModificationTime(); + long life = currentTime - time; + + if (LOG.isTraceEnabled()) { + LOG.trace("Log life:" + life + ", ttl:" + ttl + ", current:" + currentTime + ", from: " + + time); } - long life = currentTime - time; if (life < 0) { - LOG.warn("Found a log newer than current time, " + - "probably a clock skew"); + LOG.warn("Found a log (" + fStat.getPath() + ") newer than current time (" + currentTime + + " < " + time + "), probably a clock skew"); return false; } return life > ttl; Index: hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotHFileCleaner.java =================================================================== --- hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotHFileCleaner.java (revision 1505831) +++ hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotHFileCleaner.java (working copy) @@ -25,6 +25,7 @@ import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.master.cleaner.BaseHFileCleanerDelegate; @@ -54,12 +55,12 @@ private SnapshotFileCache cache; @Override - public synchronized boolean isFileDeletable(Path filePath) { + public synchronized boolean isFileDeletable(FileStatus fStat) { try { - return !cache.contains(filePath.getName()); + return !cache.contains(fStat.getPath().getName()); } catch (IOException e) { - LOG.error("Exception while checking if:" + filePath + " was valid, keeping it just in case.", - e); + LOG.error("Exception while checking if:" + fStat.getPath() + + " was valid, keeping it just in case.", e); return false; } } Index: hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotLogCleaner.java =================================================================== --- hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotLogCleaner.java (revision 1505831) +++ hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotLogCleaner.java (working copy) @@ -25,6 +25,7 @@ import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.master.cleaner.BaseLogCleanerDelegate; @@ -53,13 +54,13 @@ private SnapshotFileCache cache; @Override - public synchronized boolean isFileDeletable(Path filePath) { + public synchronized boolean isFileDeletable(FileStatus fStat) { try { if (null == cache) return false; - return !cache.contains(filePath.getName()); + return !cache.contains(fStat.getPath().getName()); } catch (IOException e) { - LOG.error("Exception while checking if:" + filePath + " was valid, keeping it just in case.", - e); + LOG.error("Exception while checking if:" + fStat.getPath() + + " was valid, keeping it just in case.", e); return false; } } Index: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationLogCleaner.java =================================================================== --- hbase-server/src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationLogCleaner.java (revision 1505831) +++ hbase-server/src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationLogCleaner.java (working copy) @@ -22,6 +22,7 @@ import org.apache.commons.logging.LogFactory; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.Abortable; import org.apache.hadoop.hbase.HConstants; @@ -52,14 +53,14 @@ @Override - public boolean isLogDeletable(Path filePath) { + public boolean isLogDeletable(FileStatus fStat) { // all members of this class are null if replication is disabled, and we // return true since false would render the LogsCleaner useless if (this.getConf() == null) { return true; } - String log = filePath.getName(); + String log = fStat.getPath().getName(); // If we saw the hlog previously, let's consider it's still used // At some point in the future we will refresh the list and it will be gone if (this.hlogs.contains(log)) { Index: hbase-server/src/test/java/org/apache/hadoop/hbase/backup/example/TestZooKeeperTableArchiveClient.java =================================================================== --- hbase-server/src/test/java/org/apache/hadoop/hbase/backup/example/TestZooKeeperTableArchiveClient.java (revision 1505831) +++ hbase-server/src/test/java/org/apache/hadoop/hbase/backup/example/TestZooKeeperTableArchiveClient.java (working copy) @@ -343,7 +343,7 @@ if (counter[0] >= expected) finished.countDown(); return ret; } - }).when(delegateSpy).isFileDeletable(Mockito.any(Path.class)); + }).when(delegateSpy).isFileDeletable(Mockito.any(FileStatus.class)); cleaners.set(0, delegateSpy); return finished; Index: hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java =================================================================== --- hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java (revision 1505831) +++ hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java (working copy) @@ -25,6 +25,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.HBaseTestingUtility; @@ -139,13 +140,15 @@ Path parent = new Path(testDir, "parent"); Path file = new Path(parent, "someFile"); fs.mkdirs(parent); + assertTrue("Test parent didn't get created.", fs.exists(parent)); // touch a new file fs.create(file).close(); assertTrue("Test file didn't get created.", fs.exists(file)); - + + FileStatus fStat = fs.getFileStatus(parent); chore.chore(); // make sure we never checked the directory - Mockito.verify(spy, Mockito.never()).isFileDeletable(parent); + Mockito.verify(spy, Mockito.never()).isFileDeletable(fStat); Mockito.reset(spy); } @@ -212,7 +215,7 @@ FSUtils.logFileSystemState(fs, testDir, LOG); return (Boolean) invocation.callRealMethod(); } - }).when(spy).isFileDeletable(Mockito.any(Path.class)); + }).when(spy).isFileDeletable(Mockito.any(FileStatus.class)); // run the chore chore.chore(); @@ -221,7 +224,7 @@ assertTrue("Added file unexpectedly deleted", fs.exists(addedFile)); assertTrue("Parent directory deleted unexpectedly", fs.exists(parent)); assertFalse("Original file unexpectedly retained", fs.exists(file)); - Mockito.verify(spy, Mockito.times(1)).isFileDeletable(Mockito.any(Path.class)); + Mockito.verify(spy, Mockito.times(1)).isFileDeletable(Mockito.any(FileStatus.class)); Mockito.reset(spy); } @@ -270,7 +273,7 @@ FSUtils.logFileSystemState(fs, testDir, LOG); return (Boolean) invocation.callRealMethod(); } - }).when(spy).isFileDeletable(Mockito.any(Path.class)); + }).when(spy).isFileDeletable(Mockito.any(FileStatus.class)); // attempt to delete the directory, which if (chore.checkAndDeleteDirectory(parent)) { @@ -282,7 +285,7 @@ assertTrue("Added file unexpectedly deleted", fs.exists(racyFile)); assertTrue("Parent directory deleted unexpectedly", fs.exists(parent)); assertFalse("Original file unexpectedly retained", fs.exists(file)); - Mockito.verify(spy, Mockito.times(1)).isFileDeletable(Mockito.any(Path.class)); + Mockito.verify(spy, Mockito.times(1)).isFileDeletable(Mockito.any(FileStatus.class)); } private static class AllValidPaths extends CleanerChore { @@ -301,14 +304,14 @@ public static class AlwaysDelete extends BaseHFileCleanerDelegate { @Override - public boolean isFileDeletable(Path file) { + public boolean isFileDeletable(FileStatus fStat) { return true; } } public static class NeverDelete extends BaseHFileCleanerDelegate { @Override - public boolean isFileDeletable(Path file) { + public boolean isFileDeletable(FileStatus fStat) { return false; } } Index: hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestHFileCleaner.java =================================================================== --- hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestHFileCleaner.java (revision 1505831) +++ hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestHFileCleaner.java (working copy) @@ -75,7 +75,7 @@ conf.setLong(TimeToLiveHFileCleaner.TTL_CONF_KEY, 100); cleaner.setConf(conf); assertTrue("File not set deletable - check mod time:" + getFileStats(file, fs) - + " with create time:" + createTime, cleaner.isFileDeletable(file)); + + " with create time:" + createTime, cleaner.isFileDeletable(fs.getFileStatus(file))); } /** Index: hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotHFileCleaner.java =================================================================== --- hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotHFileCleaner.java (revision 1505831) +++ hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotHFileCleaner.java (working copy) @@ -84,6 +84,6 @@ fs.createNewFile(new Path(archivedHfileDir, hfile)); // make sure that the file isn't deletable - assertFalse(cleaner.isFileDeletable(new Path(hfile))); + assertFalse(cleaner.isFileDeletable(fs.getFileStatus(refFile))); } } \ No newline at end of file Index: hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotLogCleaner.java =================================================================== --- hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotLogCleaner.java (revision 1505831) +++ hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotLogCleaner.java (working copy) @@ -80,6 +80,6 @@ fs.create(logFile); // make sure that the file isn't deletable - assertFalse(cleaner.isFileDeletable(logFile)); + assertFalse(cleaner.isFileDeletable(fs.getFileStatus(logFile))); } } \ No newline at end of file