From a5853221b4bc7314573a3f8c2d64c69a3c96eb78 Mon Sep 17 00:00:00 2001 From: Chia-Ping Tsai Date: Fri, 1 Dec 2017 16:09:59 +0800 Subject: [PATCH] HBASE-19339 Eager policy results in the negative size of memstore --- .../org/apache/hadoop/hbase/regionserver/HRegion.java | 11 +++++------ .../org/apache/hadoop/hbase/regionserver/HStore.java | 4 ++-- .../hadoop/hbase/regionserver/StoreFlushContext.java | 8 +++----- .../hbase/TestAcidGuaranteesWithAdaptivePolicy.java | 6 +----- .../hbase/TestAcidGuaranteesWithEagerPolicy.java | 7 +------ .../TestAcidGuaranteesWithNoInMemCompaction.java | 8 +++++++- 6 files changed, 19 insertions(+), 25 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index 364c32ad944..e66ad59e050 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -2451,13 +2451,10 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi } for (HStore s : storesToFlush) { - MemStoreSize flushableSize = s.getFlushableSize(); - totalSizeOfFlushableStores.incMemStoreSize(flushableSize); storeFlushCtxs.put(s.getColumnFamilyDescriptor().getName(), s.createFlushContext(flushOpSeqId, tracker)); // for writing stores to WAL committedFiles.put(s.getColumnFamilyDescriptor().getName(), null); - storeFlushableSize.put(s.getColumnFamilyDescriptor().getName(), flushableSize); } // write the snapshot start to WAL @@ -2470,9 +2467,11 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi } // Prepare flush (take a snapshot) - for (StoreFlushContext flush : storeFlushCtxs.values()) { - flush.prepare(); - } + storeFlushCtxs.forEach((name, flush) -> { + MemStoreSize snapshotSize = flush.prepare(); + totalSizeOfFlushableStores.incMemStoreSize(snapshotSize); + storeFlushableSize.put(name, snapshotSize); + }); } catch (IOException ex) { doAbortFlushToWAL(wal, flushOpSeqId, committedFiles); throw ex; 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 2b23598629c..80f91c8b68e 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 @@ -48,7 +48,6 @@ import java.util.function.Predicate; import java.util.function.ToLongFunction; import java.util.stream.Collectors; import java.util.stream.LongStream; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; @@ -2185,12 +2184,13 @@ public class HStore implements Store, HeapSize, StoreConfigInformation, Propagat * If necessary, the lock can be added with the patch provided in HBASE-10087 */ @Override - public void prepare() { + public MemStoreSize prepare() { // passing the current sequence number of the wal - to allow bookkeeping in the memstore this.snapshot = memstore.snapshot(); this.cacheFlushCount = snapshot.getCellsCount(); this.cacheFlushSize = snapshot.getDataSize(); committedFiles = new ArrayList<>(1); + return new MemStoreSize(snapshot.getDataSize(), snapshot.getHeapSize()); } @Override diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFlushContext.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFlushContext.java index 0db0bf147bc..ec48879e440 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFlushContext.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFlushContext.java @@ -20,10 +20,9 @@ package org.apache.hadoop.hbase.regionserver; import java.io.IOException; import java.util.List; - import org.apache.hadoop.fs.Path; -import org.apache.yetus.audience.InterfaceAudience; import org.apache.hadoop.hbase.monitoring.MonitoredTask; +import org.apache.yetus.audience.InterfaceAudience; /** * A package protected interface for a store flushing. @@ -34,12 +33,11 @@ interface StoreFlushContext { /** * Prepare for a store flush (create snapshot) - * * Requires pausing writes. - * * A very short operation. + * @return The size of snapshot to flush */ - void prepare(); + MemStoreSize prepare(); /** * Flush the cache (create the new store file) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestAcidGuaranteesWithAdaptivePolicy.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestAcidGuaranteesWithAdaptivePolicy.java index 3bc417e7019..91500cb3439 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestAcidGuaranteesWithAdaptivePolicy.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestAcidGuaranteesWithAdaptivePolicy.java @@ -18,14 +18,10 @@ */ package org.apache.hadoop.hbase; -import org.apache.hadoop.hbase.testclassification.FlakeyTests; import org.apache.hadoop.hbase.testclassification.MediumTests; -import org.junit.Ignore; import org.junit.experimental.categories.Category; -@Category({ FlakeyTests.class, MediumTests.class }) -// TODO: HBASE-19266 disables this test as the adaptive policy causes the negative size of MemStore -@Ignore +@Category({ MediumTests.class }) public class TestAcidGuaranteesWithAdaptivePolicy extends TestAcidGuaranteesWithNoInMemCompaction { @Override diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestAcidGuaranteesWithEagerPolicy.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestAcidGuaranteesWithEagerPolicy.java index b2c87f6d4df..ccbdb3483d9 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestAcidGuaranteesWithEagerPolicy.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestAcidGuaranteesWithEagerPolicy.java @@ -18,16 +18,11 @@ */ package org.apache.hadoop.hbase; -import org.apache.hadoop.hbase.testclassification.FlakeyTests; import org.apache.hadoop.hbase.testclassification.MediumTests; -import org.junit.Ignore; import org.junit.experimental.categories.Category; -@Category({ FlakeyTests.class, MediumTests.class }) -// TODO: HBASE-19266 disables this test as the eager policy causes the negative size of MemStore -@Ignore +@Category({ MediumTests.class }) public class TestAcidGuaranteesWithEagerPolicy extends TestAcidGuaranteesWithNoInMemCompaction { - @Override protected MemoryCompactionPolicy getMemoryCompactionPolicy() { return MemoryCompactionPolicy.EAGER; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestAcidGuaranteesWithNoInMemCompaction.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestAcidGuaranteesWithNoInMemCompaction.java index 9ce2337df53..df46702cdee 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestAcidGuaranteesWithNoInMemCompaction.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestAcidGuaranteesWithNoInMemCompaction.java @@ -34,8 +34,10 @@ import org.junit.After; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; +import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.junit.rules.TestRule; import org.apache.hadoop.hbase.shaded.com.google.common.collect.Lists; @@ -46,7 +48,11 @@ import org.apache.hadoop.hbase.shaded.com.google.common.collect.Lists; */ @Category({ MediumTests.class }) public class TestAcidGuaranteesWithNoInMemCompaction { - + @Rule + public final TestRule timeout = CategoryBasedTimeout.builder() + .withTimeout(this.getClass()) + .withLookingForStuckThread(true) + .build(); private static final HBaseTestingUtility UTIL = new HBaseTestingUtility(); private AcidGuaranteesTestTool tool = new AcidGuaranteesTestTool();