You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ha...@apache.org on 2021/05/24 14:19:13 UTC
[hbase] branch branch-2 updated: HBASE-25899 Improve efficiency of
SnapshotHFileCleaner (#3280)
This is an automated email from the ASF dual-hosted git repository.
haxiaolin pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2 by this push:
new 76d56bc HBASE-25899 Improve efficiency of SnapshotHFileCleaner (#3280)
76d56bc is described below
commit 76d56bc42e270f77af896c849908ad32092f4d84
Author: Xiaolin Ha <ha...@apache.org>
AuthorDate: Mon May 24 22:15:47 2021 +0800
HBASE-25899 Improve efficiency of SnapshotHFileCleaner (#3280)
Signed-off-by: Duo Zhang <zh...@apache.org>
---
.../hbase/master/snapshot/SnapshotFileCache.java | 48 ++++++++++++----------
.../master/snapshot/SnapshotHFileCleaner.java | 2 +-
2 files changed, 27 insertions(+), 23 deletions(-)
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 fe46e4c..dc91515 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,10 +20,8 @@ package org.apache.hadoop.hbase.master.snapshot;
import java.io.IOException;
import java.util.Collection;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.List;
import java.util.Map;
-import java.util.Set;
import java.util.Timer;
import java.util.TimerTask;
import java.util.concurrent.locks.Lock;
@@ -36,6 +34,8 @@ 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.CommonFSUtils;
+import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableMap;
+import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableSet;
import org.apache.yetus.audience.InterfaceAudience;
import org.apache.yetus.audience.InterfaceStability;
import org.slf4j.Logger;
@@ -91,12 +91,12 @@ public class SnapshotFileCache implements Stoppable {
private final FileSystem fs, workingFs;
private final SnapshotFileInspector fileInspector;
private final Path snapshotDir, workingSnapshotDir;
- private final Set<String> cache = new HashSet<>();
+ private volatile ImmutableSet<String> cache = ImmutableSet.of();
/**
* This is a helper map of information about the snapshot directories so we don't need to rescan
* them if they haven't changed since the last time we looked.
*/
- private final Map<String, SnapshotDirectoryInfo> snapshots = new HashMap<>();
+ private ImmutableMap<String, SnapshotDirectoryInfo> snapshots = ImmutableMap.of();
private final Timer refreshTimer;
/**
@@ -184,7 +184,7 @@ public class SnapshotFileCache implements Stoppable {
// XXX this is inefficient to synchronize on the method, when what we really need to guard against
// is an illegal access to the cache. Really we could do a mutex-guarded pointer swap on the
// cache, but that seems overkill at the moment and isn't necessarily a bottleneck.
- public synchronized Iterable<FileStatus> getUnreferencedFiles(Iterable<FileStatus> files,
+ public Iterable<FileStatus> getUnreferencedFiles(Iterable<FileStatus> files,
final SnapshotManager snapshotManager) throws IOException {
List<FileStatus> unReferencedFiles = Lists.newArrayList();
List<String> snapshotsInProgress = null;
@@ -200,13 +200,17 @@ public class SnapshotFileCache implements Stoppable {
"skip to clean the HFiles this time");
return unReferencedFiles;
}
+ ImmutableSet<String> currentCache = cache;
for (FileStatus file : files) {
String fileName = file.getPath().getName();
- if (!refreshed && !cache.contains(fileName)) {
- refreshCache();
- refreshed = true;
+ if (!refreshed && !currentCache.contains(fileName)) {
+ synchronized (this) {
+ refreshCache();
+ currentCache = cache;
+ refreshed = true;
+ }
}
- if (cache.contains(fileName)) {
+ if (currentCache.contains(fileName)) {
continue;
}
if (snapshotsInProgress == null) {
@@ -235,22 +239,23 @@ public class SnapshotFileCache implements Stoppable {
// clear the cache, as in the below code, either we will also clear the snapshots, or we will
// refill the file name cache again.
- this.cache.clear();
if (ArrayUtils.isEmpty(snapshotDirs)) {
// remove all the remembered snapshots because we don't have any left
if (LOG.isDebugEnabled() && this.snapshots.size() > 0) {
LOG.debug("No snapshots on-disk, clear cache");
}
- this.snapshots.clear();
+ this.snapshots = ImmutableMap.of();
+ this.cache = ImmutableSet.of();
return;
}
+ ImmutableSet.Builder<String> cacheBuilder = ImmutableSet.builder();
+ ImmutableMap.Builder<String, SnapshotDirectoryInfo> snapshotsBuilder = ImmutableMap.builder();
// iterate over all the cached snapshots and see if we need to update some, it is not an
// expensive operation if we do not reload the manifest of snapshots.
- Map<String, SnapshotDirectoryInfo> newSnapshots = new HashMap<>();
for (FileStatus snapshotDir : snapshotDirs) {
String name = snapshotDir.getPath().getName();
- SnapshotDirectoryInfo files = this.snapshots.remove(name);
+ SnapshotDirectoryInfo files = snapshots.get(name);
// if we don't know about the snapshot or its been modified, we need to update the
// files the latter could occur where I create a snapshot, then delete it, and then make a
// new snapshot with the same name. We will need to update the cache the information from
@@ -262,19 +267,20 @@ public class SnapshotFileCache implements Stoppable {
files = new SnapshotDirectoryInfo(snapshotDir.getModificationTime(), storedFiles);
}
// add all the files to cache
- this.cache.addAll(files.getFiles());
- newSnapshots.put(name, files);
+ cacheBuilder.addAll(files.getFiles());
+ snapshotsBuilder.put(name, files);
}
// set the snapshots we are tracking
- this.snapshots.clear();
- this.snapshots.putAll(newSnapshots);
+ this.snapshots = snapshotsBuilder.build();
+ this.cache = cacheBuilder.build();
}
List<String> getSnapshotsInProgress() throws IOException {
List<String> snapshotInProgress = Lists.newArrayList();
// only add those files to the cache, but not to the known snapshots
- FileStatus[] snapshotsInProgress = CommonFSUtils.listStatus(this.workingFs, this.workingSnapshotDir);
+ FileStatus[] snapshotsInProgress = CommonFSUtils.listStatus(this.workingFs,
+ this.workingSnapshotDir);
if (!ArrayUtils.isEmpty(snapshotsInProgress)) {
for (FileStatus snapshot : snapshotsInProgress) {
@@ -301,11 +307,10 @@ public class SnapshotFileCache implements Stoppable {
} catch (IOException e) {
LOG.warn("Failed to refresh snapshot hfile cache!", e);
// clear all the cached entries if we meet an error
- cache.clear();
- snapshots.clear();
+ cache = ImmutableSet.of();
+ snapshots = ImmutableMap.of();
}
}
-
}
}
@@ -315,7 +320,6 @@ public class SnapshotFileCache implements Stoppable {
this.stop = true;
this.refreshTimer.cancel();
}
-
}
@Override
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotHFileCleaner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotHFileCleaner.java
index e3999a5..ac7a1f4 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotHFileCleaner.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotHFileCleaner.java
@@ -63,7 +63,7 @@ public class SnapshotHFileCleaner extends BaseHFileCleanerDelegate {
private MasterServices master;
@Override
- public synchronized Iterable<FileStatus> getDeletableFiles(Iterable<FileStatus> files) {
+ public Iterable<FileStatus> getDeletableFiles(Iterable<FileStatus> files) {
try {
return cache.getUnreferencedFiles(files, master.getSnapshotManager());
} catch (CorruptedSnapshotException cse) {