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 522b1c93f29..1524ecd744e 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 @@ -34,7 +34,6 @@ import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; 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.yetus.audience.InterfaceAudience; @@ -42,7 +41,6 @@ import org.apache.yetus.audience.InterfaceStability; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; import org.apache.hbase.thirdparty.com.google.common.collect.Lists; /** @@ -182,7 +180,6 @@ public class SnapshotFileCache implements Stoppable { final SnapshotManager snapshotManager) throws IOException { List unReferencedFiles = Lists.newArrayList(); - List snapshotsInProgress = null; boolean refreshed = false; Lock lock = null; if (snapshotManager != null) { @@ -204,12 +201,6 @@ public class SnapshotFileCache implements Stoppable { if (cache.contains(fileName)) { continue; } - if (snapshotsInProgress == null) { - snapshotsInProgress = getSnapshotsInProgress(); - } - if (snapshotsInProgress.contains(fileName)) { - continue; - } unReferencedFiles.add(file); } } finally { @@ -286,31 +277,6 @@ public class SnapshotFileCache implements Stoppable { this.snapshots.putAll(known); } - @VisibleForTesting - List getSnapshotsInProgress() 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); - FileStatus[] running = FSUtils.listStatus(fs, snapshotTmpDir); - if (running != null) { - for (FileStatus run : running) { - try { - snapshotInProgress.addAll(fileInspector.filesUnderSnapshot(run.getPath())); - } catch (CorruptedSnapshotException e) { - // See HBASE-16464 - if (e.getCause() instanceof FileNotFoundException) { - // If the snapshot is corrupt, we will delete it - fs.delete(run.getPath(), true); - LOG.warn("delete the " + run.getPath() + " due to exception:", e.getCause()); - } else { - throw e; - } - } - } - } - return snapshotInProgress; - } - /** * Simple helper task that just periodically attempts to refresh the cache */ 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 d74b906a905..7ef547782eb 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 @@ -17,11 +17,8 @@ */ package org.apache.hadoop.hbase.master.snapshot; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; import java.io.IOException; import java.util.ArrayList; @@ -29,13 +26,11 @@ import java.util.Arrays; import java.util.Collection; import java.util.HashSet; import java.util.List; -import java.util.concurrent.atomic.AtomicInteger; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtility; -import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.snapshot.SnapshotDescriptionUtils; import org.apache.hadoop.hbase.snapshot.SnapshotReferenceUtil; import org.apache.hadoop.hbase.snapshot.SnapshotTestingUtils.SnapshotMock; @@ -51,11 +46,6 @@ import org.junit.experimental.categories.Category; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.apache.hbase.thirdparty.com.google.common.collect.Iterables; -import org.apache.hbase.thirdparty.com.google.common.collect.Lists; - -import org.apache.hadoop.hbase.shaded.protobuf.generated.SnapshotProtos; - /** * Test that we correctly reload the cache, filter directories, etc. */ @@ -98,10 +88,8 @@ public class TestSnapshotFileCache { "test-snapshot-file-cache-refresh", new SnapshotFiles()); createAndTestSnapshotV1(cache, "snapshot1a", false, true); - createAndTestSnapshotV1(cache, "snapshot1b", true, true); createAndTestSnapshotV2(cache, "snapshot2a", false, true); - createAndTestSnapshotV2(cache, "snapshot2b", true, true); } @Test @@ -130,78 +118,6 @@ public class TestSnapshotFileCache { // Add a new non-tmp snapshot createAndTestSnapshotV1(cache, "snapshot0v1", false, false); createAndTestSnapshotV1(cache, "snapshot0v2", false, false); - - // Add a new tmp snapshot - createAndTestSnapshotV2(cache, "snapshot1", true, false); - - // Add another tmp snapshot - createAndTestSnapshotV2(cache, "snapshot2", true, false); - } - - @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 getSnapshotsInProgress() - throws IOException { - List result = super.getSnapshotsInProgress(); - count.incrementAndGet(); - return result; - } - - @Override public void triggerCacheRefreshForTesting() { - super.triggerCacheRefreshForTesting(); - } - }; - - SnapshotMock.SnapshotBuilder complete = - createAndTestSnapshotV1(cache, "snapshot", false, false); - - int countBeforeCheck = count.get(); - - FSUtils.logFileSystemState(fs, rootDir, LOG); - - List allStoreFiles = getStoreFilesForSnapshot(complete); - Iterable 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 - } - - private List getStoreFilesForSnapshot(SnapshotMock.SnapshotBuilder builder) - throws IOException { - final List allStoreFiles = Lists.newArrayList(); - SnapshotReferenceUtil - .visitReferencedFiles(UTIL.getConfiguration(), fs, builder.getSnapshotsDir(), - new SnapshotReferenceUtil.SnapshotVisitor() { - @Override public void storeFile(RegionInfo regionInfo, String familyName, - SnapshotProtos.SnapshotRegionManifest.StoreFile storeFile) throws IOException { - FileStatus status = mockStoreFile(storeFile.getName()); - allStoreFiles.add(status); - } - }); - return allStoreFiles; - } - - private FileStatus mockStoreFile(String storeFileName) { - FileStatus status = mock(FileStatus.class); - Path path = mock(Path.class); - when(path.getName()).thenReturn(storeFileName); - when(status.getPath()).thenReturn(path); - return status; } class SnapshotFiles implements SnapshotFileCache.SnapshotFileInspector { @@ -234,20 +150,12 @@ public class TestSnapshotFileCache { List files = new ArrayList<>(); for (int i = 0; i < 3; ++i) { for (Path filePath: builder.addRegion()) { - if (tmp) { - // We should be able to find all the files while the snapshot creation is in-progress - FSUtils.logFileSystemState(fs, rootDir, LOG); - assertFalse("Cache didn't find " + filePath, - contains(getNonSnapshotFiles(cache, filePath), filePath)); - } files.add(filePath); } } // Finalize the snapshot - if (!tmp) { - builder.commit(); - } + builder.commit(); // Make sure that all files are still present for (Path path: files) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotHFileCleaner.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotHFileCleaner.java index 08a68be4d19..76c2a4b8a83 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotHFileCleaner.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotHFileCleaner.java @@ -30,7 +30,6 @@ import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.TableName; -import org.apache.hadoop.hbase.snapshot.CorruptedSnapshotException; import org.apache.hadoop.hbase.snapshot.SnapshotDescriptionUtils; import org.apache.hadoop.hbase.snapshot.SnapshotReferenceUtil; import org.apache.hadoop.hbase.snapshot.SnapshotTestingUtils; @@ -141,17 +140,8 @@ public class TestSnapshotHFileCleaner { builder.addRegionV2(); builder.corruptOneRegionManifest(); - long period = Long.MAX_VALUE; - SnapshotFileCache cache = new SnapshotFileCache(fs, rootDir, period, 10000000, - "test-snapshot-file-cache-refresh", new SnapshotFiles()); - try { - cache.getSnapshotsInProgress(); - } catch (CorruptedSnapshotException cse) { - LOG.info("Expected exception " + cse); - } finally { - fs.delete(SnapshotDescriptionUtils.getWorkingSnapshotDir(rootDir, - TEST_UTIL.getConfiguration()), true); - } + fs.delete(SnapshotDescriptionUtils.getWorkingSnapshotDir(rootDir, TEST_UTIL.getConfiguration()), + true); } /** @@ -169,35 +159,7 @@ public class TestSnapshotHFileCleaner { builder.consolidate(); builder.corruptDataManifest(); - long period = Long.MAX_VALUE; - SnapshotFileCache cache = new SnapshotFileCache(fs, rootDir, period, 10000000, - "test-snapshot-file-cache-refresh", new SnapshotFiles()); - try { - cache.getSnapshotsInProgress(); - } catch (CorruptedSnapshotException cse) { - LOG.info("Expected exception " + cse); - } finally { - fs.delete(SnapshotDescriptionUtils.getWorkingSnapshotDir(rootDir, + fs.delete(SnapshotDescriptionUtils.getWorkingSnapshotDir(rootDir, TEST_UTIL.getConfiguration()), true); - } - } - - /** - * HBASE-16464 - */ - @Test - public void testMissedTmpSnapshot() throws IOException { - SnapshotTestingUtils.SnapshotMock - snapshotMock = new SnapshotTestingUtils.SnapshotMock(TEST_UTIL.getConfiguration(), fs, rootDir); - SnapshotTestingUtils.SnapshotMock.SnapshotBuilder builder = snapshotMock.createSnapshotV2( - SNAPSHOT_NAME_STR, TABLE_NAME_STR); - builder.addRegionV2(); - builder.missOneRegionSnapshotFile(); - - long period = Long.MAX_VALUE; - SnapshotFileCache cache = new SnapshotFileCache(fs, rootDir, period, 10000000, - "test-snapshot-file-cache-refresh", new SnapshotFiles()); - cache.getSnapshotsInProgress(); - assertFalse(fs.exists(builder.getSnapshotsDir())); } }