huaxiangsun commented on a change in pull request #1791: URL: https://github.com/apache/hbase/pull/1791#discussion_r434210245
########## File path: hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotFileCache.java ########## @@ -133,6 +145,71 @@ public void testCacheUpdatedWhenLastModifiedOfSnapDirNotUpdated() throws IOExcep createAndTestSnapshotV2(cache, "snapshot2v2", true, false, true); } + @Test + public void testWeNeverCacheTmpDirAndLoadIt() throws Exception { + + final AtomicInteger count = new AtomicInteger(0); + // don't refresh the cache unless we tell it to + long period = Long.MAX_VALUE; + SnapshotFileCache cache = new SnapshotFileCache(fs, rootDir, period, 10000000, + "test-snapshot-file-cache-refresh", new SnapshotFiles()) { + @Override + List<String> getSnapshotsInProgress() + throws IOException { + List<String> result = super.getSnapshotsInProgress(); + count.incrementAndGet(); + return result; + } + + @Override public void triggerCacheRefreshForTesting() { + super.triggerCacheRefreshForTesting(); + } + }; + + SnapshotMock.SnapshotBuilder complete = + createAndTestSnapshotV1(cache, "snapshot", false, false, false); + + int countBeforeCheck = count.get(); + + CommonFSUtils.logFileSystemState(fs, rootDir, LOG); + + List<FileStatus> allStoreFiles = getStoreFilesForSnapshot(complete); + Iterable<FileStatus> deletableFiles = cache.getUnreferencedFiles(allStoreFiles, null); + assertTrue(Iterables.isEmpty(deletableFiles)); + // no need for tmp dir check as all files are accounted for. + assertEquals(0, count.get() - countBeforeCheck); + + // add a random file to make sure we refresh + FileStatus randomFile = mockStoreFile(UTIL.getRandomUUID().toString()); + allStoreFiles.add(randomFile); + deletableFiles = cache.getUnreferencedFiles(allStoreFiles, null); + assertEquals(randomFile, Iterables.getOnlyElement(deletableFiles)); + assertEquals(1, count.get() - countBeforeCheck); // we check the tmp directory Review comment: The newly added cases in testLoadAndDelete() + createAndTestSnapshotV1(cache, "snapshot1b", true, true, false); + createAndTestSnapshotV2(cache, "snapshot1a", true, true, false); test the case that files are correctly included for snapshot in progress. Is that not enough? ---------------------------------------------------------------- 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