From 2f88f266ff8860922095c15bf03b14b2323c386a Mon Sep 17 00:00:00 2001 From: Guanghao Zhang Date: Fri, 14 Dec 2018 18:16:51 +0800 Subject: [PATCH] HBASE-21604 Move the memstore chunk creator to HRegionServer's member variable --- .../hbase/regionserver/AbstractMemStore.java | 21 +++-- .../hbase/regionserver/CompactingMemStore.java | 9 +-- .../hbase/regionserver/CompactionPipeline.java | 7 +- .../hadoop/hbase/regionserver/DefaultMemStore.java | 10 +-- .../apache/hadoop/hbase/regionserver/HRegion.java | 7 +- .../hadoop/hbase/regionserver/HRegionServer.java | 13 +++- .../hbase/regionserver/MemStoreCompactor.java | 16 ++-- .../hadoop/hbase/regionserver/MemStoreLAB.java | 6 +- .../hadoop/hbase/regionserver/MemStoreLABImpl.java | 14 ++-- .../hbase/regionserver/RegionServerServices.java | 5 ++ .../regionserver/RegionServicesForStores.java | 5 ++ .../hadoop/hbase/regionserver/SegmentFactory.java | 90 +++++++++++----------- .../apache/hadoop/hbase/HBaseTestingUtility.java | 23 ++++++ .../hadoop/hbase/MockRegionServerServices.java | 6 ++ .../hadoop/hbase/master/MockRegionServer.java | 6 ++ .../hbase/regionserver/TestDefaultMemStore.java | 62 +++++++++------ .../hbase/regionserver/TestMemStoreChunkPool.java | 9 +-- .../hadoop/hbase/regionserver/TestMemStoreLAB.java | 9 ++- .../regionserver/TestMemstoreLABWithoutPool.java | 13 ++-- 19 files changed, 201 insertions(+), 130 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/AbstractMemStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/AbstractMemStore.java index e359925..b68cff6 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/AbstractMemStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/AbstractMemStore.java @@ -45,6 +45,7 @@ public abstract class AbstractMemStore implements MemStore { private final Configuration conf; private final CellComparator comparator; + protected final RegionServicesForStores regionServices; // active segment absorbs write operations private volatile MutableSegment active; @@ -54,8 +55,6 @@ public abstract class AbstractMemStore implements MemStore { // Used to track when to flush private volatile long timeOfOldestEdit; - protected RegionServicesForStores regionServices; - public final static long FIXED_OVERHEAD = (long) ClassSize.OBJECT + (5 * ClassSize.REFERENCE) + (2 * Bytes.SIZEOF_LONG); // snapshotId, timeOfOldestEdit @@ -80,7 +79,7 @@ public abstract class AbstractMemStore implements MemStore { this.comparator = c; this.regionServices = regionServices; resetActive(); - this.snapshot = SegmentFactory.instance().createImmutableSegment(c); + this.snapshot = SegmentFactory.createImmutableSegment(c); this.snapshotId = NO_SNAPSHOT_ID; } @@ -88,12 +87,15 @@ public abstract class AbstractMemStore implements MemStore { // Record the MutableSegment' heap overhead when initialing MemStoreSizing memstoreAccounting = new NonThreadSafeMemStoreSizing(); // Reset heap to not include any keys - active = SegmentFactory.instance().createMutableSegment(conf, comparator, memstoreAccounting); // regionServices can be null when testing if (regionServices != null) { - regionServices.addMemStoreSize(memstoreAccounting.getDataSize(), - memstoreAccounting.getHeapSize(), - memstoreAccounting.getOffHeapSize()); + active = SegmentFactory.createMutableSegment(conf, comparator, memstoreAccounting, + regionServices.getChunkCreator()); + regionServices + .addMemStoreSize(memstoreAccounting.getDataSize(), memstoreAccounting.getHeapSize(), + memstoreAccounting.getOffHeapSize()); + } else { + active = SegmentFactory.createMutableSegment(conf, comparator, memstoreAccounting); } timeOfOldestEdit = Long.MAX_VALUE; } @@ -242,7 +244,7 @@ public abstract class AbstractMemStore implements MemStore { // create a new snapshot and let the old one go. Segment oldSnapshot = this.snapshot; if (!this.snapshot.isEmpty()) { - this.snapshot = SegmentFactory.instance().createImmutableSegment(this.comparator); + this.snapshot = SegmentFactory.createImmutableSegment(this.comparator); } this.snapshotId = NO_SNAPSHOT_ID; oldSnapshot.close(); @@ -388,4 +390,7 @@ public abstract class AbstractMemStore implements MemStore { */ protected abstract List getSegments() throws IOException; + public RegionServicesForStores getRegionServices() { + return this.regionServices; + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactingMemStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactingMemStore.java index 00d5273..26ced18 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactingMemStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactingMemStore.java @@ -105,7 +105,6 @@ public class CompactingMemStore extends AbstractMemStore { MemoryCompactionPolicy compactionPolicy) throws IOException { super(conf, c, regionServices); this.store = store; - this.regionServices = regionServices; this.pipeline = new CompactionPipeline(getRegionServices()); this.compactor = createMemStoreCompactor(compactionPolicy); if (conf.getBoolean(MemStoreLAB.USEMSLAB_KEY, MemStoreLAB.USEMSLAB_DEFAULT)) { @@ -556,7 +555,7 @@ public class CompactingMemStore extends AbstractMemStore { // however stopping here for the case of the infinite loop causing by any error LOG.warn("Multiple unsuccessful attempts to push the compaction pipeline to snapshot," + " while flushing to disk."); - this.snapshot = SegmentFactory.instance().createImmutableSegment(getComparator()); + this.snapshot = SegmentFactory.createImmutableSegment(getComparator()); break; } } @@ -569,14 +568,10 @@ public class CompactingMemStore extends AbstractMemStore { return; } else { // create composite snapshot this.snapshot = - SegmentFactory.instance().createCompositeImmutableSegment(getComparator(), segments); + SegmentFactory.createCompositeImmutableSegment(getComparator(), segments); } } - private RegionServicesForStores getRegionServices() { - return regionServices; - } - /** * The in-memory-flusher thread performs the flush asynchronously. * There is at most one thread per memstore instance. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactionPipeline.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactionPipeline.java index 216f7c3..dc1bd00 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactionPipeline.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactionPipeline.java @@ -74,7 +74,7 @@ public class CompactionPipeline { public boolean pushHead(MutableSegment segment) { // Record the ImmutableSegment' heap overhead when initialing MemStoreSizing memstoreAccounting = new NonThreadSafeMemStoreSizing(); - ImmutableSegment immutableSegment = SegmentFactory.instance(). + ImmutableSegment immutableSegment = SegmentFactory. createImmutableSegment(segment, memstoreAccounting); if (region != null) { region.addMemStoreSize(memstoreAccounting.getDataSize(), @@ -218,8 +218,9 @@ public class CompactionPipeline { s.waitForUpdates(); // to ensure all updates preceding s in-memory flush have completed // size to be updated MemStoreSizing newMemstoreAccounting = new NonThreadSafeMemStoreSizing(); - ImmutableSegment newS = SegmentFactory.instance().createImmutableSegmentByFlattening( - (CSLMImmutableSegment)s,idxType,newMemstoreAccounting,action); + ImmutableSegment newS = SegmentFactory + .createImmutableSegmentByFlattening((CSLMImmutableSegment) s, idxType, + newMemstoreAccounting, action); replaceAtIndex(i,newS); if(region != null) { // Update the global memstore size counter upon flattening there is no change in the diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java index a006ecb..758fd9f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java @@ -70,14 +70,6 @@ public class DefaultMemStore extends AbstractMemStore { * Constructor. * @param c Comparator */ - public DefaultMemStore(final Configuration conf, final CellComparator c) { - super(conf, c, null); - } - - /** - * Constructor. - * @param c Comparator - */ public DefaultMemStore(final Configuration conf, final CellComparator c, final RegionServicesForStores regionServices) { super(conf, c, regionServices); @@ -99,7 +91,7 @@ public class DefaultMemStore extends AbstractMemStore { if (!getActive().isEmpty()) { // Record the ImmutableSegment' heap overhead when initialing MemStoreSizing memstoreAccounting = new NonThreadSafeMemStoreSizing(); - ImmutableSegment immutableSegment = SegmentFactory.instance(). + ImmutableSegment immutableSegment = SegmentFactory. createImmutableSegment(getActive(), memstoreAccounting); // regionServices can be null when testing if (regionServices != null) { 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 21458c4..f91cc91 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 @@ -298,7 +298,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi // Track data size in all memstores private final MemStoreSizing memStoreSizing = new ThreadSafeMemStoreSizing(); - private final RegionServicesForStores regionServicesForStores = new RegionServicesForStores(this); + private RegionServicesForStores regionServicesForStores = new RegionServicesForStores(this); // Debug possible data loss due to WAL off final LongAdder numMutationsWithoutWAL = new LongAdder(); @@ -1343,6 +1343,11 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi return regionServicesForStores; } + @VisibleForTesting + public void setRegionServicesForStores(RegionServicesForStores regionServicesForStores) { + this.regionServicesForStores = regionServicesForStores; + } + @Override public long getNumMutationsWithoutWAL() { return numMutationsWithoutWAL.sum(); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java index 13f277b..8a33ff3 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java @@ -416,6 +416,8 @@ public class HRegionServer extends HasThread implements // The cache for mob files private MobFileCache mobFileCache; + private ChunkCreator chunkCreator; + /** The health check chore. */ private HealthCheckChore healthCheckChore; @@ -1603,9 +1605,9 @@ public class HRegionServer extends HasThread implements MemStoreLAB.POOL_INITIAL_SIZE_DEFAULT); int chunkSize = conf.getInt(MemStoreLAB.CHUNK_SIZE_KEY, MemStoreLAB.CHUNK_SIZE_DEFAULT); // init the chunkCreator - ChunkCreator chunkCreator = - ChunkCreator.initialize(chunkSize, offheap, globalMemStoreSize, poolSizePercentage, - initialCountPercentage, this.hMemManager); + this.chunkCreator = ChunkCreator + .initialize(chunkSize, offheap, globalMemStoreSize, poolSizePercentage, + initialCountPercentage, this.hMemManager); } } @@ -3843,4 +3845,9 @@ public class HRegionServer extends HasThread implements System.exit(1); } } + + @Override + public ChunkCreator getChunkCreator() { + return this.chunkCreator; + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactor.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactor.java index 2dafcee..b0bc7c1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactor.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactor.java @@ -218,10 +218,11 @@ public class MemStoreCompactor { iterator = new MemStoreCompactorSegmentsIterator(segments, compactingMemStore.getComparator(), compactionKVMax, compactingMemStore.getStore()); - - result = SegmentFactory.instance().createImmutableSegmentByCompaction( - compactingMemStore.getConfiguration(), compactingMemStore.getComparator(), iterator, - versionedList.getNumOfCells(), compactingMemStore.getIndexType(), action); + result = SegmentFactory + .createImmutableSegmentByCompaction(compactingMemStore.getConfiguration(), + compactingMemStore.getRegionServices().getChunkCreator(), + compactingMemStore.getComparator(), iterator, versionedList.getNumOfCells(), + compactingMemStore.getIndexType(), action); iterator.close(); break; case MERGE: @@ -229,10 +230,9 @@ public class MemStoreCompactor { iterator = new MemStoreMergerSegmentsIterator(segments, compactingMemStore.getComparator(), compactionKVMax); - - result = SegmentFactory.instance().createImmutableSegmentByMerge( - compactingMemStore.getConfiguration(), compactingMemStore.getComparator(), iterator, - versionedList.getNumOfCells(), segments, compactingMemStore.getIndexType(), action); + result = SegmentFactory.createImmutableSegmentByMerge(compactingMemStore.getConfiguration(), + compactingMemStore.getComparator(), iterator, versionedList.getNumOfCells(), segments, + compactingMemStore.getIndexType(), action); iterator.close(); break; default: diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreLAB.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreLAB.java index 90cf932..2895aa6 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreLAB.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreLAB.java @@ -110,12 +110,14 @@ public interface MemStoreLAB { */ Chunk getNewExternalChunk(int size); - static MemStoreLAB newInstance(Configuration conf) { + static MemStoreLAB newInstance(Configuration conf, ChunkCreator chunkCreator) { MemStoreLAB memStoreLAB = null; if (isEnabled(conf)) { + assert chunkCreator != null; String className = conf.get(MSLAB_CLASS_NAME, MemStoreLABImpl.class.getName()); memStoreLAB = ReflectionUtils.instantiateWithCustomCtor(className, - new Class[] { Configuration.class }, new Object[] { conf }); + new Class[] { Configuration.class, ChunkCreator.class }, + new Object[] { conf, chunkCreator }); } return memStoreLAB; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreLABImpl.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreLABImpl.java index 6f1ee92..9e2a751 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreLABImpl.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreLABImpl.java @@ -36,8 +36,10 @@ import org.apache.hadoop.hbase.KeyValueUtil; import org.apache.yetus.audience.InterfaceAudience; 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.base.Preconditions; + /** * A memstore-local allocation buffer. *

@@ -74,7 +76,7 @@ public class MemStoreLABImpl implements MemStoreLAB { // A set of chunks contained by this memstore LAB @VisibleForTesting - Set chunks = new ConcurrentSkipListSet(); + Set chunks = new ConcurrentSkipListSet<>(); private final int dataChunkSize; private final int maxAlloc; private final ChunkCreator chunkCreator; @@ -91,13 +93,13 @@ public class MemStoreLABImpl implements MemStoreLAB { // Used in testing public MemStoreLABImpl() { - this(new Configuration()); + this(new Configuration(), null); } - public MemStoreLABImpl(Configuration conf) { + public MemStoreLABImpl(Configuration conf, ChunkCreator chunkCreator) { dataChunkSize = conf.getInt(CHUNK_SIZE_KEY, CHUNK_SIZE_DEFAULT); maxAlloc = conf.getInt(MAX_ALLOC_KEY, MAX_ALLOC_DEFAULT); - this.chunkCreator = ChunkCreator.getInstance(); + this.chunkCreator = chunkCreator; // if we don't exclude allocations >CHUNK_SIZE, we'd infiniteloop on one! Preconditions.checkArgument(maxAlloc <= dataChunkSize, MAX_ALLOC_KEY + " must be less than " + CHUNK_SIZE_KEY); @@ -373,8 +375,8 @@ public class MemStoreLABImpl implements MemStoreLAB { */ @Override public Chunk getNewExternalChunk(int size) { - int allocSize = size + ChunkCreator.getInstance().SIZEOF_CHUNK_HEADER; - if (allocSize <= ChunkCreator.getInstance().getChunkSize()) { + int allocSize = size + chunkCreator.SIZEOF_CHUNK_HEADER; + if (allocSize <= chunkCreator.getChunkSize()) { return getNewExternalChunk(ChunkCreator.ChunkType.DATA_CHUNK); } else { Chunk c = this.chunkCreator.getJumboChunk(size); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java index e0638ac..ebea016 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java @@ -279,4 +279,9 @@ public interface RegionServerServices extends Server, MutableOnlineRegions, Favo * @return The cache for mob files. */ Optional getMobFileCache(); + + /* + * @return the chunk creator used by regionserver. + */ + ChunkCreator getChunkCreator(); } \ No newline at end of file diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServicesForStores.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServicesForStores.java index 31f2d85..20b5d67 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServicesForStores.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServicesForStores.java @@ -18,6 +18,7 @@ */ package org.apache.hadoop.hbase.regionserver; +import java.util.Optional; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadFactory; import java.util.concurrent.ThreadPoolExecutor; @@ -83,4 +84,8 @@ public class RegionServicesForStores { long getMemStoreSize() { return region.getMemStoreDataSize(); } + + ChunkCreator getChunkCreator() { + return this.region.getRegionServerServices().getChunkCreator(); + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SegmentFactory.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SegmentFactory.java index 26b7ecc..3a25c1d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SegmentFactory.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SegmentFactory.java @@ -27,37 +27,27 @@ import java.util.ArrayList; import java.util.List; /** - * A singleton store segment factory. * Generate concrete store segments. */ @InterfaceAudience.Private public final class SegmentFactory { - private SegmentFactory() {} - private static SegmentFactory instance = new SegmentFactory(); - - public static SegmentFactory instance() { - return instance; - } - // create composite immutable segment from a list of segments // for snapshot consisting of multiple segments - public CompositeImmutableSegment createCompositeImmutableSegment( + public static CompositeImmutableSegment createCompositeImmutableSegment( final CellComparator comparator, List segments) { return new CompositeImmutableSegment(comparator, segments); } // create new flat immutable segment from compacting old immutable segments // for compaction - public ImmutableSegment createImmutableSegmentByCompaction(final Configuration conf, - final CellComparator comparator, MemStoreSegmentsIterator iterator, int numOfCells, - CompactingMemStore.IndexType idxType, MemStoreCompactionStrategy.Action action) - throws IOException { - - MemStoreLAB memStoreLAB = MemStoreLAB.newInstance(conf); - return - createImmutableSegment( - conf,comparator,iterator,memStoreLAB,numOfCells,action,idxType); + public static ImmutableSegment createImmutableSegmentByCompaction(final Configuration conf, + ChunkCreator chunkCreator, final CellComparator comparator, MemStoreSegmentsIterator iterator, + int numOfCells, CompactingMemStore.IndexType idxType, + MemStoreCompactionStrategy.Action action) { + MemStoreLAB memStoreLAB = MemStoreLAB.newInstance(conf, chunkCreator); + return createImmutableSegment(conf, comparator, iterator, memStoreLAB, numOfCells, action, + idxType); } /** @@ -65,67 +55,72 @@ public final class SegmentFactory { * This ImmutableSegment is used as a place holder for snapshot in Memstore. * It won't flush later, So it is not necessary to record the initial size * for it. + * * @param comparator comparator * @return ImmutableSegment */ - public ImmutableSegment createImmutableSegment(CellComparator comparator) { + public static ImmutableSegment createImmutableSegment(CellComparator comparator) { MutableSegment segment = generateMutableSegment(null, comparator, null, null); return createImmutableSegment(segment, null); } // create not-flat immutable segment from mutable segment - public ImmutableSegment createImmutableSegment(MutableSegment segment, + public static ImmutableSegment createImmutableSegment(MutableSegment segment, MemStoreSizing memstoreSizing) { return new CSLMImmutableSegment(segment, memstoreSizing); } // create mutable segment - public MutableSegment createMutableSegment(final Configuration conf, + public static MutableSegment createMutableSegment(final Configuration conf, CellComparator comparator, MemStoreSizing memstoreSizing) { - MemStoreLAB memStoreLAB = MemStoreLAB.newInstance(conf); + return generateMutableSegment(conf, comparator, null, memstoreSizing); + } + + // create mutable segment + public static MutableSegment createMutableSegment(final Configuration conf, + CellComparator comparator, MemStoreSizing memstoreSizing, ChunkCreator chunkCreator) { + MemStoreLAB memStoreLAB = MemStoreLAB.newInstance(conf, chunkCreator); return generateMutableSegment(conf, comparator, memStoreLAB, memstoreSizing); } // create new flat immutable segment from merging old immutable segments // for merge - public ImmutableSegment createImmutableSegmentByMerge(final Configuration conf, + public static ImmutableSegment createImmutableSegmentByMerge(final Configuration conf, final CellComparator comparator, MemStoreSegmentsIterator iterator, int numOfCells, List segments, CompactingMemStore.IndexType idxType, - MemStoreCompactionStrategy.Action action) - throws IOException { + MemStoreCompactionStrategy.Action action) throws IOException { MemStoreLAB memStoreLAB = getMergedMemStoreLAB(conf, segments); - return - createImmutableSegment( - conf,comparator,iterator,memStoreLAB,numOfCells,action,idxType); + return createImmutableSegment(conf, comparator, iterator, memStoreLAB, numOfCells, action, + idxType); } // create flat immutable segment from non-flat immutable segment // for flattening - public ImmutableSegment createImmutableSegmentByFlattening( - CSLMImmutableSegment segment, CompactingMemStore.IndexType idxType, - MemStoreSizing memstoreSizing, MemStoreCompactionStrategy.Action action) { + public static ImmutableSegment createImmutableSegmentByFlattening(CSLMImmutableSegment segment, + CompactingMemStore.IndexType idxType, MemStoreSizing memstoreSizing, + MemStoreCompactionStrategy.Action action) { ImmutableSegment res = null; switch (idxType) { - case CHUNK_MAP: - res = new CellChunkImmutableSegment(segment, memstoreSizing, action); - break; - case CSLM_MAP: - assert false; // non-flat segment can not be the result of flattening - break; - case ARRAY_MAP: - res = new CellArrayImmutableSegment(segment, memstoreSizing, action); - break; + case CHUNK_MAP: + res = new CellChunkImmutableSegment(segment, memstoreSizing, action); + break; + case CSLM_MAP: + assert false; // non-flat segment can not be the result of flattening + break; + case ARRAY_MAP: + res = new CellArrayImmutableSegment(segment, memstoreSizing, action); + break; } return res; } - //****** private methods to instantiate concrete store segments **********// - private ImmutableSegment createImmutableSegment(final Configuration conf, final CellComparator comparator, - MemStoreSegmentsIterator iterator, MemStoreLAB memStoreLAB, int numOfCells, - MemStoreCompactionStrategy.Action action, CompactingMemStore.IndexType idxType) { + private static ImmutableSegment createImmutableSegment(final Configuration conf, + final CellComparator comparator, MemStoreSegmentsIterator iterator, MemStoreLAB memStoreLAB, + int numOfCells, MemStoreCompactionStrategy.Action action, + CompactingMemStore.IndexType idxType) { ImmutableSegment res = null; switch (idxType) { @@ -142,14 +137,15 @@ public final class SegmentFactory { return res; } - private MutableSegment generateMutableSegment(final Configuration conf, CellComparator comparator, - MemStoreLAB memStoreLAB, MemStoreSizing memstoreSizing) { + private static MutableSegment generateMutableSegment(final Configuration conf, + CellComparator comparator, MemStoreLAB memStoreLAB, MemStoreSizing memstoreSizing) { // TBD use configuration to set type of segment CellSet set = new CellSet(comparator); return new MutableSegment(set, comparator, memStoreLAB, memstoreSizing); } - private MemStoreLAB getMergedMemStoreLAB(Configuration conf, List segments) { + private static MemStoreLAB getMergedMemStoreLAB(Configuration conf, + List segments) { List mslabs = new ArrayList<>(); if (!conf.getBoolean(MemStoreLAB.USEMSLAB_KEY, MemStoreLAB.USEMSLAB_DEFAULT)) { return null; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java index 796dbc3..b1d291d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java @@ -117,6 +117,7 @@ import org.apache.hadoop.hbase.regionserver.Region; import org.apache.hadoop.hbase.regionserver.RegionScanner; import org.apache.hadoop.hbase.regionserver.RegionServerServices; import org.apache.hadoop.hbase.regionserver.RegionServerStoppedException; +import org.apache.hadoop.hbase.regionserver.RegionServicesForStores; import org.apache.hadoop.hbase.security.HBaseKerberosUtils; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.visibility.VisibilityLabelsCache; @@ -2585,6 +2586,20 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility { region.initialize(); return region; } + + /** + * Create a region with it's own WAL. Be sure to call + * {@link HBaseTestingUtility#closeRegionAndWAL(HRegion)} to clean up all resources. + */ + public static HRegion createRegionAndWAL(final RegionInfo info, final Path rootDir, + final Configuration conf, final TableDescriptor htd, + RegionServicesForStores regionServicesForStores) throws IOException { + HRegion region = createRegionAndWAL(info, rootDir, conf, htd, false); + region.setRegionServicesForStores(regionServicesForStores); + region.initialize(); + return region; + } + /** * Create a region with it's own WAL. Be sure to call * {@link HBaseTestingUtility#closeRegionAndWAL(HRegion)} to clean up all resources. @@ -4122,6 +4137,14 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility { } public HRegion createTestRegion(String tableName, ColumnFamilyDescriptor cd, + RegionServicesForStores regionServicesForStores) throws IOException { + TableDescriptor td = + TableDescriptorBuilder.newBuilder(TableName.valueOf(tableName)).setColumnFamily(cd).build(); + RegionInfo info = RegionInfoBuilder.newBuilder(TableName.valueOf(tableName)).build(); + return createRegionAndWAL(info, getDataTestDir(), getConfiguration(), td, regionServicesForStores); + } + + public HRegion createTestRegion(String tableName, ColumnFamilyDescriptor cd, BlockCache blockCache) throws IOException { TableDescriptor td = TableDescriptorBuilder.newBuilder(TableName.valueOf(tableName)).setColumnFamily(cd).build(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java index 0e4f241..03c571c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java @@ -43,6 +43,7 @@ import org.apache.hadoop.hbase.mob.MobFileCache; import org.apache.hadoop.hbase.quotas.RegionServerRpcQuotaManager; import org.apache.hadoop.hbase.quotas.RegionServerSpaceQuotaManager; import org.apache.hadoop.hbase.quotas.RegionSizeStore; +import org.apache.hadoop.hbase.regionserver.ChunkCreator; import org.apache.hadoop.hbase.regionserver.FlushRequester; import org.apache.hadoop.hbase.regionserver.HRegion; import org.apache.hadoop.hbase.regionserver.HeapMemoryManager; @@ -368,4 +369,9 @@ public class MockRegionServerServices implements RegionServerServices { public Optional getMobFileCache() { return Optional.empty(); } + + @Override + public ChunkCreator getChunkCreator() { + return null; + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java index a930d7f..3766bb7 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java @@ -56,6 +56,7 @@ import org.apache.hadoop.hbase.mob.MobFileCache; import org.apache.hadoop.hbase.quotas.RegionServerRpcQuotaManager; import org.apache.hadoop.hbase.quotas.RegionServerSpaceQuotaManager; import org.apache.hadoop.hbase.quotas.RegionSizeStore; +import org.apache.hadoop.hbase.regionserver.ChunkCreator; import org.apache.hadoop.hbase.regionserver.FlushRequester; import org.apache.hadoop.hbase.regionserver.HRegion; import org.apache.hadoop.hbase.regionserver.HeapMemoryManager; @@ -721,4 +722,9 @@ class MockRegionServer implements AdminProtos.AdminService.BlockingInterface, public Optional getMobFileCache() { return Optional.empty(); } + + @Override + public ChunkCreator getChunkCreator() { + return null; + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java index 77f796f..875d02b 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java @@ -21,6 +21,9 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.when; import java.io.IOException; import java.util.ArrayList; @@ -32,6 +35,7 @@ import java.util.concurrent.atomic.AtomicReference; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.Cell; +import org.apache.hadoop.hbase.CellComparator; import org.apache.hadoop.hbase.CellComparatorImpl; import org.apache.hadoop.hbase.CellUtil; import org.apache.hadoop.hbase.HBaseClassTestRule; @@ -60,11 +64,13 @@ import org.apache.hadoop.hbase.util.FSTableDescriptors; import org.apache.hadoop.hbase.wal.WALFactory; import org.junit.AfterClass; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; import org.junit.rules.TestName; +import org.mockito.Mockito; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -81,6 +87,11 @@ public class TestDefaultMemStore { HBaseClassTestRule.forClass(TestDefaultMemStore.class); private static final Logger LOG = LoggerFactory.getLogger(TestDefaultMemStore.class); + + private static Configuration conf; + + private static RegionServicesForStores regionServices; + @Rule public TestName name = new TestName(); protected AbstractMemStore memstore; protected static final int ROW_COUNT = 10; @@ -94,13 +105,21 @@ public class TestDefaultMemStore { return this.name.getMethodName(); } + @BeforeClass + public static void sttupClass() throws Exception { + conf = HBaseConfiguration.create(); + regionServices = Mockito.mock(RegionServicesForStores.class); + doNothing().when(regionServices).addMemStoreSize(anyLong(), anyLong(), anyLong()); + } + @Before public void setUp() throws Exception { internalSetUp(); // no pool this.chunkCreator = ChunkCreator.initialize(MemStoreLABImpl.CHUNK_SIZE_DEFAULT, false, 0, 0, 0, null); - this.memstore = new DefaultMemStore(); + when(regionServices.getChunkCreator()).thenReturn(this.chunkCreator); + this.memstore = new DefaultMemStore(conf, CellComparator.getInstance(), regionServices); } @AfterClass @@ -259,7 +278,7 @@ public class TestDefaultMemStore { verifyScanAcrossSnapshot2(kv1, kv2); // use case 3: first in snapshot second in kvset - this.memstore = new DefaultMemStore(); + this.memstore = new DefaultMemStore(conf, CellComparator.getInstance(), regionServices); this.memstore.add(kv1.clone(), null); this.memstore.snapshot(); this.memstore.add(kv2.clone(), null); @@ -539,23 +558,22 @@ public class TestDefaultMemStore { @Test public void testMultipleVersionsSimple() throws Exception { - DefaultMemStore m = new DefaultMemStore(new Configuration(), CellComparatorImpl.COMPARATOR); - byte [] row = Bytes.toBytes("testRow"); - byte [] family = Bytes.toBytes("testFamily"); - byte [] qf = Bytes.toBytes("testQualifier"); - long [] stamps = {1,2,3}; - byte [][] values = {Bytes.toBytes("value0"), Bytes.toBytes("value1"), - Bytes.toBytes("value2")}; + byte[] row = Bytes.toBytes("testRow"); + byte[] family = Bytes.toBytes("testFamily"); + byte[] qf = Bytes.toBytes("testQualifier"); + long[] stamps = { 1, 2, 3 }; + byte[][] values = { Bytes.toBytes("value0"), Bytes.toBytes("value1"), Bytes.toBytes("value2") }; KeyValue key0 = new KeyValue(row, family, qf, stamps[0], values[0]); KeyValue key1 = new KeyValue(row, family, qf, stamps[1], values[1]); KeyValue key2 = new KeyValue(row, family, qf, stamps[2], values[2]); - m.add(key0, null); - m.add(key1, null); - m.add(key2, null); + memstore.add(key0, null); + memstore.add(key1, null); + memstore.add(key2, null); - assertTrue("Expected memstore to hold 3 values, actually has " + - m.getActive().getCellsCount(), m.getActive().getCellsCount() == 3); + assertTrue( + "Expected memstore to hold 3 values, actually has " + memstore.getActive().getCellsCount(), + memstore.getActive().getCellsCount() == 3); } ////////////////////////////////////////////////////////////////////////////// @@ -821,8 +839,6 @@ public class TestDefaultMemStore { */ @Test public void testUpsertMemstoreSize() throws Exception { - Configuration conf = HBaseConfiguration.create(); - memstore = new DefaultMemStore(conf, CellComparatorImpl.COMPARATOR); MemStoreSize oldSize = memstore.size(); List l = new ArrayList<>(); @@ -846,7 +862,6 @@ public class TestDefaultMemStore { assertEquals(newSize, this.memstore.size()); //The kv2 should be removed. assert(memstore.getActive().getCellsCount() == 2); - //this.memstore = null; } //////////////////////////////////// @@ -864,7 +879,6 @@ public class TestDefaultMemStore { try { EnvironmentEdgeForMemstoreTest edge = new EnvironmentEdgeForMemstoreTest(); EnvironmentEdgeManager.injectEdge(edge); - DefaultMemStore memstore = new DefaultMemStore(); long t = memstore.timeOfOldestEdit(); assertEquals(Long.MAX_VALUE, t); @@ -918,8 +932,8 @@ public class TestDefaultMemStore { EnvironmentEdgeManager.injectEdge(edge); HBaseTestingUtility hbaseUtility = HBaseTestingUtility.createLocalHTU(conf); String cf = "foo"; - HRegion region = - hbaseUtility.createTestRegion("foobar", ColumnFamilyDescriptorBuilder.of(cf)); + HRegion region = hbaseUtility + .createTestRegion("foobar", ColumnFamilyDescriptorBuilder.of(cf), regionServices); edge.setCurrentTimeMillis(1234); Put p = new Put(Bytes.toBytes("r")); @@ -950,14 +964,18 @@ public class TestDefaultMemStore { WALFactory wFactory = new WALFactory(conf, "1234"); HRegion meta = HRegion.createHRegion(RegionInfoBuilder.FIRST_META_REGIONINFO, testDir, conf, FSTableDescriptors.createMetaTableDescriptor(conf), - wFactory.getWAL(RegionInfoBuilder.FIRST_META_REGIONINFO)); + wFactory.getWAL(RegionInfoBuilder.FIRST_META_REGIONINFO), false); + meta.setRegionServicesForStores(regionServices); + meta.initialize(); // parameterized tests add [#] suffix get rid of [ and ]. TableDescriptor desc = TableDescriptorBuilder .newBuilder(TableName.valueOf(name.getMethodName().replaceAll("[\\[\\]]", "_"))) .setColumnFamily(ColumnFamilyDescriptorBuilder.of("foo")).build(); RegionInfo hri = RegionInfoBuilder.newBuilder(desc.getTableName()) .setStartKey(Bytes.toBytes("row_0200")).setEndKey(Bytes.toBytes("row_0300")).build(); - HRegion r = HRegion.createHRegion(hri, testDir, conf, desc, wFactory.getWAL(hri)); + HRegion r = HRegion.createHRegion(hri, testDir, conf, desc, wFactory.getWAL(hri), false); + r.setRegionServicesForStores(regionServices); + r.initialize(); addRegionToMETA(meta, r); edge.setCurrentTimeMillis(1234 + 100); StringBuilder sb = new StringBuilder(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStoreChunkPool.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStoreChunkPool.java index 4f3de36..9699268 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStoreChunkPool.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStoreChunkPool.java @@ -42,9 +42,6 @@ import org.junit.ClassRule; import org.junit.Test; import org.junit.experimental.categories.Category; -/** - * Test the {@link MemStoreChunkPool} class - */ @Category({RegionServerTests.class, SmallTests.class}) public class TestMemStoreChunkPool { @@ -82,7 +79,7 @@ public class TestMemStoreChunkPool { @Test public void testReusingChunks() { Random rand = new Random(); - MemStoreLAB mslab = new MemStoreLABImpl(conf); + MemStoreLAB mslab = new MemStoreLABImpl(conf, chunkCreator); int expectedOff = 0; ByteBuffer lastBuffer = null; final byte[] rk = Bytes.toBytes("r1"); @@ -108,7 +105,7 @@ public class TestMemStoreChunkPool { int chunkCount = chunkCreator.getPoolSize(); assertTrue(chunkCount > 0); // reconstruct mslab - mslab = new MemStoreLABImpl(conf); + mslab = new MemStoreLABImpl(conf, chunkCreator); // chunk should be got from the pool, so we can reuse it. KeyValue kv = new KeyValue(rk, cf, q, new byte[10]); mslab.copyCellInto(kv); @@ -245,7 +242,7 @@ public class TestMemStoreChunkPool { Runnable r = new Runnable() { @Override public void run() { - MemStoreLAB memStoreLAB = new MemStoreLABImpl(conf); + MemStoreLAB memStoreLAB = new MemStoreLABImpl(conf, chunkCreator); for (int i = 0; i < maxCount; i++) { memStoreLAB.copyCellInto(kv);// Try allocate size = chunkSize. Means every // allocate call will result in a new chunk diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStoreLAB.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStoreLAB.java index ef4ad69..bd493f5 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStoreLAB.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStoreLAB.java @@ -63,10 +63,13 @@ public class TestMemStoreLAB { private static final byte[] cf = Bytes.toBytes("f"); private static final byte[] q = Bytes.toBytes("q"); + private static ChunkCreator chunkCreator; + @BeforeClass public static void setUpBeforeClass() throws Exception { - ChunkCreator.initialize(1 * 1024, false, 50 * 1024000L, 0.2f, - MemStoreLAB.POOL_INITIAL_SIZE_DEFAULT, null); + chunkCreator = ChunkCreator + .initialize(1 * 1024, false, 50 * 1024000L, 0.2f, MemStoreLAB.POOL_INITIAL_SIZE_DEFAULT, + null); } @AfterClass @@ -219,7 +222,7 @@ public class TestMemStoreLAB { ChunkCreator.initialize(MemStoreLABImpl.MAX_ALLOC_DEFAULT, false, globalMemStoreLimit, 0.1f, MemStoreLAB.POOL_INITIAL_SIZE_DEFAULT, null); ChunkCreator.clearDisableFlag(); - mslab = new MemStoreLABImpl(conf); + mslab = new MemStoreLABImpl(conf, chunkCreator); // launch multiple threads to trigger frequent chunk retirement List threads = new ArrayList<>(); final KeyValue kv = new KeyValue(Bytes.toBytes("r"), Bytes.toBytes("f"), Bytes.toBytes("q"), diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemstoreLABWithoutPool.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemstoreLABWithoutPool.java index e2da5d0..5a40221 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemstoreLABWithoutPool.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemstoreLABWithoutPool.java @@ -55,13 +55,16 @@ public class TestMemstoreLABWithoutPool { private static final byte[] cf = Bytes.toBytes("f"); private static final byte[] q = Bytes.toBytes("q"); + private static ChunkCreator chunkCreator; + @BeforeClass public static void setUpBeforeClass() throws Exception { - long globalMemStoreLimit = (long) (ManagementFactory.getMemoryMXBean().getHeapMemoryUsage() - .getMax() * 0.8); + long globalMemStoreLimit = + (long) (ManagementFactory.getMemoryMXBean().getHeapMemoryUsage().getMax() * 0.8); // disable pool - ChunkCreator.initialize(MemStoreLABImpl.CHUNK_SIZE_DEFAULT + Bytes.SIZEOF_LONG, false, globalMemStoreLimit, - 0.0f, MemStoreLAB.POOL_INITIAL_SIZE_DEFAULT, null); + chunkCreator = ChunkCreator + .initialize(MemStoreLABImpl.CHUNK_SIZE_DEFAULT + Bytes.SIZEOF_LONG, false, + globalMemStoreLimit, 0.0f, MemStoreLAB.POOL_INITIAL_SIZE_DEFAULT, null); } /** @@ -108,7 +111,7 @@ public class TestMemstoreLABWithoutPool { Configuration conf = HBaseConfiguration.create(); MemStoreLABImpl[] mslab = new MemStoreLABImpl[10]; for (int i = 0; i < 10; i++) { - mslab[i] = new MemStoreLABImpl(conf); + mslab[i] = new MemStoreLABImpl(conf, chunkCreator); } // launch multiple threads to trigger frequent chunk retirement List threads = new ArrayList<>(); -- 2.7.4