diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotFileCache.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotFileCache.java index 358b4ea..e4e4645 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotFileCache.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotFileCache.java @@ -20,8 +20,10 @@ package org.apache.hadoop.hbase.master.snapshot; import java.io.FileNotFoundException; import java.io.IOException; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; @@ -37,6 +39,7 @@ import org.apache.hadoop.hbase.Stoppable; import org.apache.hadoop.hbase.snapshot.CorruptedSnapshotException; import org.apache.hadoop.hbase.snapshot.SnapshotDescriptionUtils; import org.apache.hadoop.hbase.util.FSUtils; +import org.apache.hadoop.hbase.util.Pair; import org.apache.yetus.audience.InterfaceAudience; import org.apache.yetus.audience.InterfaceStability; import org.slf4j.Logger; @@ -44,6 +47,7 @@ import org.slf4j.LoggerFactory; import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; import org.apache.hbase.thirdparty.com.google.common.collect.Lists; +import org.apache.hbase.thirdparty.com.google.common.collect.Sets; /** * Intelligently keep track of all the files for all the snapshots. @@ -184,18 +188,35 @@ public class SnapshotFileCache implements Stoppable { List unReferencedFiles = Lists.newArrayList(); List snapshotsInProgress = null; boolean refreshed = false; - for (FileStatus file : files) { + boolean needToCheckInProgressSnapshots = false; + Set snapshotNamesInProgressFromCacheRefresh = Collections.emptySet(); + Iterator iter = files.iterator(); + while (iter.hasNext()) { + FileStatus file = iter.next(); String fileName = file.getPath().getName(); if (!refreshed && !cache.contains(fileName)) { - refreshCache(); - refreshed = true; + snapshotNamesInProgressFromCacheRefresh = refreshCache(); + needToCheckInProgressSnapshots = refreshed = true; } if (cache.contains(fileName)) { continue; } + Set namesInProgress = Collections.emptySet(); if (snapshotsInProgress == null) { - snapshotsInProgress = getSnapshotsInProgress(snapshotManager); + Pair, List> pair = getSnapshotsInProgress(snapshotManager); + namesInProgress = pair.getFirst(); + snapshotsInProgress = pair.getSecond(); } + if (needToCheckInProgressSnapshots && + !namesInProgress.equals(snapshotNamesInProgressFromCacheRefresh)) { + LOG.debug("# snapshot names InProgress: " + namesInProgress.size() + + " # snapshot Names InProgress from refreshCache: " + + snapshotNamesInProgressFromCacheRefresh.size()); + // since there is discrepancy w.r.t. the number of in progress snapshots + // keep files for this round + return unReferencedFiles; + } + needToCheckInProgressSnapshots = false; if (snapshotsInProgress.contains(fileName)) { continue; } @@ -204,7 +225,7 @@ public class SnapshotFileCache implements Stoppable { return unReferencedFiles; } - private synchronized void refreshCache() throws IOException { + private synchronized Set refreshCache() throws IOException { // get the status of the snapshots directory and check if it is has changes FileStatus dirStatus; try { @@ -213,11 +234,13 @@ public class SnapshotFileCache implements Stoppable { if (this.cache.size() > 0) { LOG.error("Snapshot directory: " + snapshotDir + " doesn't exist"); } - return; + return getSnapshotNamesInProgress(); } // if the snapshot directory wasn't modified since we last check, we are done - if (dirStatus.getModificationTime() <= this.lastModifiedTime) return; + if (dirStatus.getModificationTime() <= this.lastModifiedTime) { + return getSnapshotNamesInProgress(); + } // directory was modified, so we need to reload our cache // there could be a slight race here where we miss the cache, check the directory modification @@ -239,7 +262,7 @@ public class SnapshotFileCache implements Stoppable { LOG.debug("No snapshots on-disk, cache empty"); } this.snapshots.clear(); - return; + return getSnapshotNamesInProgress(); } // 3.1 iterate through the on-disk snapshots @@ -267,23 +290,40 @@ public class SnapshotFileCache implements Stoppable { // 4. set the snapshots we are tracking this.snapshots.clear(); this.snapshots.putAll(known); + return getSnapshotNamesInProgress(); } - @VisibleForTesting List getSnapshotsInProgress( + private Set getSnapshotNamesInProgress() throws IOException { + Path snapshotTmpDir = new Path(snapshotDir, SnapshotDescriptionUtils.SNAPSHOT_TMP_DIR_NAME); + FileStatus[] running = FSUtils.listStatus(fs, snapshotTmpDir); + if (running != null) { + Set snapshotInProgress = Sets.newHashSet(); + for (FileStatus f : running) { + snapshotInProgress.add(f.getPath().getName()); + } + return snapshotInProgress; + } + return Collections.emptySet(); + } + @VisibleForTesting + Pair, List> getSnapshotsInProgress( final SnapshotManager snapshotManager) throws IOException { List snapshotInProgress = Lists.newArrayList(); // only add those files to the cache, but not to the known snapshots Path snapshotTmpDir = new Path(snapshotDir, SnapshotDescriptionUtils.SNAPSHOT_TMP_DIR_NAME); // only add those files to the cache, but not to the known snapshots FileStatus[] running = FSUtils.listStatus(fs, snapshotTmpDir); + Set names = Sets.newHashSet(); if (running != null) { for (FileStatus run : running) { + boolean success = false; ReentrantLock lock = null; if (snapshotManager != null) { lock = snapshotManager.getLocks().acquireLock(run.getPath().getName()); } try { snapshotInProgress.addAll(fileInspector.filesUnderSnapshot(run.getPath())); + success = true; } catch (CorruptedSnapshotException e) { // See HBASE-16464 if (e.getCause() instanceof FileNotFoundException) { @@ -298,9 +338,12 @@ public class SnapshotFileCache implements Stoppable { lock.unlock(); } } + if (success) { + names.add(run.getPath().getName()); + } } } - return snapshotInProgress; + return new Pair<>(names, snapshotInProgress); } /** diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotFileCache.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotFileCache.java index 22d2734..9241aa5 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotFileCache.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotFileCache.java @@ -29,6 +29,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; @@ -42,6 +43,7 @@ import org.apache.hadoop.hbase.snapshot.SnapshotTestingUtils.SnapshotMock; import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.util.FSUtils; +import org.apache.hadoop.hbase.util.Pair; import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -147,9 +149,9 @@ public class TestSnapshotFileCache { SnapshotFileCache cache = new SnapshotFileCache(fs, rootDir, period, 10000000, "test-snapshot-file-cache-refresh", new SnapshotFiles()) { @Override - List getSnapshotsInProgress(final SnapshotManager snapshotManager) + Pair, List> getSnapshotsInProgress(final SnapshotManager snapshotManager) throws IOException { - List result = super.getSnapshotsInProgress(snapshotManager); + Pair, List> result = super.getSnapshotsInProgress(snapshotManager); count.incrementAndGet(); return result; }