You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by te...@apache.org on 2018/11/26 02:20:37 UTC
hbase git commit: HBASE-21511 Remove in progress snapshot check in
SnapshotFileCache#getUnreferencedFiles
Repository: hbase
Updated Branches:
refs/heads/master 701526d19 -> 27c0bf5c6
HBASE-21511 Remove in progress snapshot check in SnapshotFileCache#getUnreferencedFiles
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/27c0bf5c
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/27c0bf5c
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/27c0bf5c
Branch: refs/heads/master
Commit: 27c0bf5c6373e805db8552b9aae46a887f5cc0a9
Parents: 701526d
Author: Ted Yu <yu...@gmail.com>
Authored: Sun Nov 25 18:20:00 2018 -0800
Committer: Ted Yu <yu...@gmail.com>
Committed: Sun Nov 25 18:20:00 2018 -0800
----------------------------------------------------------------------
.../master/snapshot/SnapshotFileCache.java | 34 -------
.../master/snapshot/TestSnapshotFileCache.java | 94 +-------------------
.../snapshot/TestSnapshotHFileCleaner.java | 44 +--------
3 files changed, 4 insertions(+), 168 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/27c0bf5c/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotFileCache.java
----------------------------------------------------------------------
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 522b1c9..1524ecd 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<FileStatus> unReferencedFiles = Lists.newArrayList();
- List<String> 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<String> getSnapshotsInProgress() throws IOException {
- List<String> 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
*/
http://git-wip-us.apache.org/repos/asf/hbase/blob/27c0bf5c/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotFileCache.java
----------------------------------------------------------------------
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 d74b906..7ef5477 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<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);
-
- int countBeforeCheck = count.get();
-
- FSUtils.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
- }
-
- private List<FileStatus> getStoreFilesForSnapshot(SnapshotMock.SnapshotBuilder builder)
- throws IOException {
- final List<FileStatus> 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<Path> 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) {
http://git-wip-us.apache.org/repos/asf/hbase/blob/27c0bf5c/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotHFileCleaner.java
----------------------------------------------------------------------
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 08a68be..76c2a4b 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()));
}
}