From f6e9d3e1dd6546663319c436286ab58dd0e731ed Mon Sep 17 00:00:00 2001 From: Wellington Ramos Chevreuil Date: Mon, 9 May 2022 10:24:47 +0100 Subject: [PATCH] =?UTF-8?q?HBASE-26999=20HStore=20should=20try=20write=20W?= =?UTF-8?q?AL=20compaction=20marker=20before=20repl=E2=80=A6=20(#4407)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Duo Zhang Signed-off-by: Andrew Purtell --- .../org/apache/hadoop/hbase/regionserver/HStore.java | 11 +++++++---- .../apache/hadoop/hbase/regionserver/StoreEngine.java | 8 +++++--- .../apache/hadoop/hbase/regionserver/TestHStore.java | 4 ++-- 3 files changed, 14 insertions(+), 9 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 3d6a9e9fc62..d4482dd44ba 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 @@ -1232,14 +1232,17 @@ public class HStore allowedOnPath = ".*/(HStore|TestHStore).java") void replaceStoreFiles(Collection compactedFiles, Collection result, boolean writeCompactionMarker) throws IOException { - storeEngine.replaceStoreFiles(compactedFiles, result, () -> { + storeEngine.replaceStoreFiles(compactedFiles, result, + () -> { + if (writeCompactionMarker) { + writeCompactionWalRecord(compactedFiles, result); + } + }, + () -> { synchronized (filesCompacting) { filesCompacting.removeAll(compactedFiles); } }); - if (writeCompactionMarker) { - writeCompactionWalRecord(compactedFiles, result); - } // 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 91fe44c350c..a9863ea8451 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 @@ -34,6 +34,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.Function; + import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.CellComparator; @@ -407,8 +408,7 @@ public abstract class StoreEngine 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 } /** @@ -491,9 +491,11 @@ public abstract class StoreEngine compactedFiles, - Collection newFiles, Runnable actionUnderLock) throws IOException { + Collection newFiles, IOExceptionRunnable walMarkerWriter, + Runnable actionUnderLock) throws IOException { storeFileTracker.replace(StoreUtils.toStoreFileInfo(compactedFiles), StoreUtils.toStoreFileInfo(newFiles)); + walMarkerWriter.run(); writeLock(); try { storeFileManager.addCompactionResults(compactedFiles, newFiles); 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 da5a124e6e5..d8dd6b21b78 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 @@ -1017,14 +1017,14 @@ public class TestHStore { // call first time after files changed spiedStoreEngine.refreshStoreFiles(); assertEquals(2, this.store.getStorefilesCount()); - verify(spiedStoreEngine, times(1)).replaceStoreFiles(any(), any(), any()); + verify(spiedStoreEngine, times(1)).replaceStoreFiles(any(), 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(), any()); + verify(spiedStoreEngine, times(1)).replaceStoreFiles(any(), any(), any(), any()); } private long countMemStoreScanner(StoreScanner scanner) {