You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zh...@apache.org on 2022/01/19 08:59:52 UTC
[hbase] branch branch-2 updated: HBASE-26674 Should modify filesCompacting under storeWriteLock (#4040)
This is an automated email from the ASF dual-hosted git repository.
zhangduo 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 8fbc9a2 HBASE-26674 Should modify filesCompacting under storeWriteLock (#4040)
8fbc9a2 is described below
commit 8fbc9a2606a91325da0eb0955ca924236cc6e71f
Author: Duo Zhang <zh...@apache.org>
AuthorDate: Wed Jan 19 13:59:35 2022 +0800
HBASE-26674 Should modify filesCompacting under storeWriteLock (#4040)
Signed-off-by: Josh Elser <el...@apache.org>
---
.../main/java/org/apache/hadoop/hbase/regionserver/HStore.java | 9 +++++----
.../java/org/apache/hadoop/hbase/regionserver/StoreEngine.java | 6 ++++--
.../java/org/apache/hadoop/hbase/regionserver/TestHStore.java | 4 ++--
3 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
index e910f3c..41f1838 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
@@ -1229,13 +1229,14 @@ public class HStore implements Store, HeapSize, StoreConfigInformation,
allowedOnPath = ".*/(HStore|TestHStore).java")
void replaceStoreFiles(Collection<HStoreFile> compactedFiles, Collection<HStoreFile> result,
boolean writeCompactionMarker) throws IOException {
- storeEngine.replaceStoreFiles(compactedFiles, result);
+ storeEngine.replaceStoreFiles(compactedFiles, result, () -> {
+ synchronized(filesCompacting) {
+ filesCompacting.removeAll(compactedFiles);
+ }
+ });
if (writeCompactionMarker) {
writeCompactionWalRecord(compactedFiles, result);
}
- synchronized (filesCompacting) {
- filesCompacting.removeAll(compactedFiles);
- }
// These may be null when the RS is shutting down. The space quota Chores will fix the Region
// sizes later so it's not super-critical if we miss these.
RegionServerServices rsServices = region.getRegionServerServices();
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreEngine.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreEngine.java
index ddb52d1..d85553a 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreEngine.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreEngine.java
@@ -410,7 +410,8 @@ public abstract class StoreEngine<SF extends StoreFlusher, CP extends Compaction
List<HStoreFile> openedFiles = openStoreFiles(toBeAddedFiles, false);
// propogate the file changes to the underlying store file manager
- replaceStoreFiles(toBeRemovedStoreFiles, openedFiles); // won't throw an exception
+ replaceStoreFiles(toBeRemovedStoreFiles, openedFiles, () -> {
+ }); // won't throw an exception
}
/**
@@ -493,12 +494,13 @@ public abstract class StoreEngine<SF extends StoreFlusher, CP extends Compaction
}
public void replaceStoreFiles(Collection<HStoreFile> compactedFiles,
- Collection<HStoreFile> newFiles) throws IOException {
+ Collection<HStoreFile> newFiles, Runnable actionUnderLock) throws IOException {
storeFileTracker.replace(StoreUtils.toStoreFileInfo(compactedFiles),
StoreUtils.toStoreFileInfo(newFiles));
writeLock();
try {
storeFileManager.addCompactionResults(compactedFiles, newFiles);
+ actionUnderLock.run();
} finally {
writeUnlock();
}
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStore.java
index bdee770..0440a8c 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStore.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStore.java
@@ -1034,14 +1034,14 @@ public class TestHStore {
// call first time after files changed
spiedStoreEngine.refreshStoreFiles();
assertEquals(2, this.store.getStorefilesCount());
- verify(spiedStoreEngine, times(1)).replaceStoreFiles(any(), any());
+ verify(spiedStoreEngine, times(1)).replaceStoreFiles(any(), any(), any());
// call second time
spiedStoreEngine.refreshStoreFiles();
// ensure that replaceStoreFiles is not called, i.e, the times does not change, if files are not
// refreshed,
- verify(spiedStoreEngine, times(1)).replaceStoreFiles(any(), any());
+ verify(spiedStoreEngine, times(1)).replaceStoreFiles(any(), any(), any());
}
private long countMemStoreScanner(StoreScanner scanner) {