HBASE-26567 Remove IndexType from ChunkCreator

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ChunkCreator.java

@@ -134,51 +134,36 @@ public static ChunkCreator getInstance() {
     return instance;
   }
 
-  /**
-   * Creates and inits a chunk. The default implementation for a specific chunk size.
-   * @return the chunk that was initialized
-   */
-  Chunk getChunk(ChunkType chunkType) {
-    return getChunk(CompactingMemStore.IndexType.ARRAY_MAP, chunkType);
-  }
 
   /**
-   * Creates and inits a chunk. The default implementation.
+   * Creates and inits a data chunk. The default implementation.
    * @return the chunk that was initialized
    */
   Chunk getChunk() {
-    return getChunk(CompactingMemStore.IndexType.ARRAY_MAP, ChunkType.DATA_CHUNK);
-  }
-
-  /**
-   * Creates and inits a chunk. The default implementation for a specific index type.
-   * @return the chunk that was initialized
-   */
-  Chunk getChunk(CompactingMemStore.IndexType chunkIndexType) {
-    return getChunk(chunkIndexType, ChunkType.DATA_CHUNK);
+    return getChunk(ChunkType.DATA_CHUNK);
   }
 
   /**
-   * Creates and inits a chunk with specific index type and type.
+   * Creates and inits a chunk with specific type.
    * @return the chunk that was initialized
    */
-  Chunk getChunk(CompactingMemStore.IndexType chunkIndexType, ChunkType chunkType) {
+  Chunk getChunk(ChunkType chunkType) {
     switch (chunkType) {
       case INDEX_CHUNK:
         if (indexChunksPool == null) {
           if (indexChunkSize <= 0) {
             throw new IllegalArgumentException(
                 "chunkType is INDEX_CHUNK but indexChunkSize is:[" + this.indexChunkSize + "]");
           }
-          return getChunk(chunkIndexType, chunkType, indexChunkSize);
+          return getChunk(chunkType, indexChunkSize);
         } else {
-          return getChunk(chunkIndexType, chunkType, indexChunksPool.getChunkSize());
+          return getChunk(chunkType, indexChunksPool.getChunkSize());
         }
       case DATA_CHUNK:
         if (dataChunksPool == null) {
-          return getChunk(chunkIndexType, chunkType, chunkSize);
+          return getChunk(chunkType, chunkSize);
         } else {
-          return getChunk(chunkIndexType, chunkType, dataChunksPool.getChunkSize());
+          return getChunk(chunkType, dataChunksPool.getChunkSize());
         }
       default:
         throw new IllegalArgumentException(
@@ -189,10 +174,9 @@ Chunk getChunk(CompactingMemStore.IndexType chunkIndexType, ChunkType chunkType)
   /**
    * Creates and inits a chunk.
    * @return the chunk that was initialized
-   * @param chunkIndexType whether the requested chunk is going to be used with CellChunkMap index
    * @param size the size of the chunk to be allocated, in bytes
    */
-  Chunk getChunk(CompactingMemStore.IndexType chunkIndexType, ChunkType chunkType, int size) {
+  Chunk getChunk(ChunkType chunkType, int size) {
     Chunk chunk = null;
     MemStoreChunkPool pool = null;
 
@@ -217,9 +201,7 @@ Chunk getChunk(CompactingMemStore.IndexType chunkIndexType, ChunkType chunkType,
     }
 
     if (chunk == null) {
-      // the second parameter explains whether CellChunkMap index is requested,
-      // in that case, put allocated on demand chunk mapping into chunkIdMap
-      chunk = createChunk(false, chunkIndexType, chunkType, size);
+      chunk = createChunk(false, chunkType, size);
     }
 
     // now we need to actually do the expensive memory allocation step in case of a new chunk,
@@ -240,11 +222,11 @@ Chunk getJumboChunk(int jumboSize) {
     if (allocSize <= this.getChunkSize(ChunkType.DATA_CHUNK)) {
       LOG.warn("Jumbo chunk size " + jumboSize + " must be more than regular chunk size "
           + this.getChunkSize(ChunkType.DATA_CHUNK) + ". Converting to regular chunk.");
-      return getChunk(CompactingMemStore.IndexType.CHUNK_MAP);
+      return getChunk();
     }
     // the new chunk is going to hold the jumbo cell data and needs to be referenced by
-    // a strong map. Therefore the CCM index type
-    return getChunk(CompactingMemStore.IndexType.CHUNK_MAP, ChunkType.JUMBO_CHUNK, allocSize);
+    // a strong map.
+    return getChunk(ChunkType.JUMBO_CHUNK, allocSize);
   }
 
   /**
@@ -254,8 +236,7 @@ Chunk getJumboChunk(int jumboSize) {
    * @param size the size of the chunk to be allocated, in bytes
    * @return the chunk
    */
-  private Chunk createChunk(boolean pool, CompactingMemStore.IndexType chunkIndexType,
-      ChunkType chunkType, int size) {
+  private Chunk createChunk(boolean pool, ChunkType chunkType, int size) {
     Chunk chunk = null;
     int id = chunkID.getAndIncrement();
     assert id > 0;
@@ -265,22 +246,20 @@ private Chunk createChunk(boolean pool, CompactingMemStore.IndexType chunkIndexT
     } else {
       chunk = new OnheapChunk(size, id, chunkType, pool);
     }
-    if (pool || (chunkIndexType == CompactingMemStore.IndexType.CHUNK_MAP)) {
-      // put the pool chunk into the chunkIdMap so it is not GC-ed
-      this.chunkIdMap.put(chunk.getId(), chunk);
-    }
+    // put the chunk into the chunkIdMap so it is not GC-ed
+    this.chunkIdMap.put(chunk.getId(), chunk);
+
     return chunk;
   }
 
-  // Chunks from pool are created covered with strong references anyway
-  // TODO: change to CHUNK_MAP if it is generally defined
-  private Chunk createChunkForPool(CompactingMemStore.IndexType chunkIndexType, ChunkType chunkType,
+  // Chunks from pool are created covered with strong references anyway.
+  private Chunk createChunkForPool(ChunkType chunkType,
       int chunkSize) {
     if (chunkSize != dataChunksPool.getChunkSize() &&
             chunkSize != indexChunksPool.getChunkSize()) {
       return null;
     }
-    return createChunk(true, chunkIndexType, chunkType, chunkSize);
+    return createChunk(true, chunkType, chunkSize);
   }
 
   // Used to translate the ChunkID into a chunk ref
@@ -346,7 +325,7 @@ private  class MemStoreChunkPool implements HeapMemoryTuneObserver {
       this.reclaimedChunks = new LinkedBlockingQueue<>();
       for (int i = 0; i < initialCount; i++) {
         Chunk chunk =
-            createChunk(true, CompactingMemStore.IndexType.ARRAY_MAP, chunkType, chunkSize);
+            createChunk(true, chunkType, chunkSize);
         chunk.init();
         reclaimedChunks.add(chunk);
       }
@@ -368,10 +347,6 @@ private  class MemStoreChunkPool implements HeapMemoryTuneObserver {
      * @see #putbackChunks(Chunk)
      */
     Chunk getChunk() {
-      return getChunk(CompactingMemStore.IndexType.ARRAY_MAP);
-    }
-
-    Chunk getChunk(CompactingMemStore.IndexType chunkIndexType) {
       Chunk chunk = reclaimedChunks.poll();
       if (chunk != null) {
         chunk.reset();
@@ -382,7 +357,7 @@ Chunk getChunk(CompactingMemStore.IndexType chunkIndexType) {
           long created = this.chunkCount.get();
           if (created < this.maxCount) {
             if (this.chunkCount.compareAndSet(created, created + 1)) {
-              chunk = createChunkForPool(chunkIndexType, chunkType, chunkSize);
+              chunk = createChunkForPool(chunkType, chunkSize);
               break;
             }
           } else {

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreLABImpl.java

@@ -77,7 +77,6 @@ public class MemStoreLABImpl implements MemStoreLAB {
   private final int dataChunkSize;
   private final int maxAlloc;
   private final ChunkCreator chunkCreator;
-  private final CompactingMemStore.IndexType idxType; // what index is used for corresponding segment
 
   // This flag is for closing this instance, its set when clearing snapshot of
   // memstore
@@ -101,9 +100,9 @@ public MemStoreLABImpl(Configuration conf) {
     Preconditions.checkArgument(maxAlloc <= dataChunkSize,
         MAX_ALLOC_KEY + " must be less than " + CHUNK_SIZE_KEY);
 
-    // if user requested to work with MSLABs (whether on- or off-heap), then the
-    // immutable segments are going to use CellChunkMap as their index
-    idxType = CompactingMemStore.IndexType.CHUNK_MAP;
+    // NOTE:if user requested to work with MSLABs (whether on- or off-heap), then the
+    // immutable segments are going to use CellChunkMap as their index,see
+    // CompactingMemStore#indexType
   }
 
   @Override
@@ -336,7 +335,7 @@ private Chunk getOrMakeChunk() {
         if (c != null) {
           return c;
         }
-        c = this.chunkCreator.getChunk(idxType);
+        c = this.chunkCreator.getChunk();
         if (c != null) {
           // set the curChunk. No need of CAS as only one thread will be here
           currChunk.set(c);

hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCellFlatSet.java

@@ -282,8 +282,8 @@ private CellChunkMap setUpCellChunkMap(boolean asc) {
 
     // allocate new chunks and use the data chunk to hold the full data of the cells
     // and the index chunk to hold the cell-representations
-    Chunk dataChunk = chunkCreator.getChunk(CompactingMemStore.IndexType.CHUNK_MAP);
-    Chunk idxChunk  = chunkCreator.getChunk(CompactingMemStore.IndexType.CHUNK_MAP);
+    Chunk dataChunk = chunkCreator.getChunk();
+    Chunk idxChunk = chunkCreator.getChunk();
     // the array of index chunks to be used as a basis for CellChunkMap
     Chunk chunkArray[] = new Chunk[8];  // according to test currently written 8 is way enough
     int chunkArrayIdx = 0;
@@ -300,7 +300,7 @@ private CellChunkMap setUpCellChunkMap(boolean asc) {
       // do we have enough space to write the cell data on the data chunk?
       if (dataOffset + kv.getSerializedSize() > chunkCreator.getChunkSize()) {
         // allocate more data chunks if needed
-        dataChunk = chunkCreator.getChunk(CompactingMemStore.IndexType.CHUNK_MAP);
+        dataChunk = chunkCreator.getChunk();
         dataBuffer = dataChunk.getData();
         dataOffset = ChunkCreator.SIZEOF_CHUNK_HEADER;
       }
@@ -310,7 +310,7 @@ private CellChunkMap setUpCellChunkMap(boolean asc) {
       // do we have enough space to write the cell-representation on the index chunk?
       if (idxOffset + ClassSize.CELL_CHUNK_MAP_ENTRY > chunkCreator.getChunkSize()) {
         // allocate more index chunks if needed
-        idxChunk = chunkCreator.getChunk(CompactingMemStore.IndexType.CHUNK_MAP);
+        idxChunk = chunkCreator.getChunk();
         idxBuffer = idxChunk.getData();
         idxOffset = ChunkCreator.SIZEOF_CHUNK_HEADER;
         chunkArray[chunkArrayIdx++] = idxChunk;
@@ -331,10 +331,10 @@ private CellChunkMap setUpJumboCellChunkMap(boolean asc) {
     // allocate new chunks and use the data JUMBO chunk to hold the full data of the cells
     // and the normal index chunk to hold the cell-representations
     Chunk dataJumboChunk =
-        chunkCreator.getChunk(CompactingMemStore.IndexType.CHUNK_MAP, ChunkType.JUMBO_CHUNK,
+        chunkCreator.getChunk(ChunkType.JUMBO_CHUNK,
           smallChunkSize);
     assertTrue(dataJumboChunk.isJumbo());
-    Chunk idxChunk  = chunkCreator.getChunk(CompactingMemStore.IndexType.CHUNK_MAP);
+    Chunk idxChunk = chunkCreator.getChunk();
     // the array of index chunks to be used as a basis for CellChunkMap
     Chunk[] chunkArray = new Chunk[8];  // according to test currently written 8 is way enough
     int chunkArrayIdx = 0;
@@ -354,7 +354,7 @@ private CellChunkMap setUpJumboCellChunkMap(boolean asc) {
       // do we have enough space to write the cell-representation on the index chunk?
       if (idxOffset + ClassSize.CELL_CHUNK_MAP_ENTRY > chunkCreator.getChunkSize()) {
         // allocate more index chunks if needed
-        idxChunk = chunkCreator.getChunk(CompactingMemStore.IndexType.CHUNK_MAP);
+        idxChunk = chunkCreator.getChunk();
         idxBuffer = idxChunk.getData();
         idxOffset = ChunkCreator.SIZEOF_CHUNK_HEADER;
         chunkArray[chunkArrayIdx++] = idxChunk;
@@ -368,7 +368,7 @@ private CellChunkMap setUpJumboCellChunkMap(boolean asc) {
       // Jumbo chunks are working only with one cell per chunk, thus always allocate a new jumbo
       // data chunk for next cell
       dataJumboChunk =
-          chunkCreator.getChunk(CompactingMemStore.IndexType.CHUNK_MAP, ChunkType.JUMBO_CHUNK,
+          chunkCreator.getChunk(ChunkType.JUMBO_CHUNK,
             smallChunkSize);
       assertTrue(dataJumboChunk.isJumbo());
       dataBuffer = dataJumboChunk.getData();

hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStoreChunkPool.java

@@ -312,7 +312,7 @@ public void testNoIndexChunksPoolOrNoDataChunksPool() throws Exception {
       assertEquals(initialCount, newCreator.getPoolSize());
       assertEquals(0, newCreator.getPoolSize(ChunkType.INDEX_CHUNK));
 
-      Chunk dataChunk = newCreator.getChunk(CompactingMemStore.IndexType.CHUNK_MAP);
+      Chunk dataChunk = newCreator.getChunk();
       assertTrue(dataChunk.isDataChunk());
       assertTrue(dataChunk.isFromPool());
       assertEquals(initialCount - 1, newCreator.getPoolSize());
@@ -323,7 +323,7 @@ public void testNoIndexChunksPoolOrNoDataChunksPool() throws Exception {
 
       // We set ChunkCreator.indexChunkSize to 0, but we want to get a IndexChunk
       try {
-        newCreator.getChunk(CompactingMemStore.IndexType.CHUNK_MAP, ChunkType.INDEX_CHUNK);
+        newCreator.getChunk(ChunkType.INDEX_CHUNK);
         fail();
       } catch (IllegalArgumentException e) {
       }
@@ -350,15 +350,15 @@ public void testNoIndexChunksPoolOrNoDataChunksPool() throws Exception {
       assertEquals(0, newCreator.getPoolSize());
       assertEquals(0, newCreator.getPoolSize(ChunkType.INDEX_CHUNK));
 
-      dataChunk = newCreator.getChunk(CompactingMemStore.IndexType.CHUNK_MAP);
+      dataChunk = newCreator.getChunk();
       assertTrue(dataChunk.isDataChunk());
       assertTrue(!dataChunk.isFromPool());
       assertEquals(0, newCreator.getPoolSize());
       assertEquals(0, newCreator.getPoolSize(ChunkType.INDEX_CHUNK));
 
       try {
         // We set ChunkCreator.indexChunkSize to 0, but we want to get a IndexChunk
-        newCreator.getChunk(CompactingMemStore.IndexType.CHUNK_MAP, ChunkType.INDEX_CHUNK);
+        newCreator.getChunk(ChunkType.INDEX_CHUNK);
         fail();
       } catch (IllegalArgumentException e) {
       }
@@ -387,14 +387,14 @@ public void testNoIndexChunksPoolOrNoDataChunksPool() throws Exception {
       assertEquals(0, newCreator.getPoolSize());
       assertEquals(initialCount, newCreator.getPoolSize(ChunkType.INDEX_CHUNK));
 
-      dataChunk = newCreator.getChunk(CompactingMemStore.IndexType.CHUNK_MAP);
+      dataChunk = newCreator.getChunk();
       assertTrue(dataChunk.isDataChunk());
       assertTrue(!dataChunk.isFromPool());
       assertEquals(0, newCreator.getPoolSize());
       assertEquals(initialCount, newCreator.getPoolSize(ChunkType.INDEX_CHUNK));
 
       Chunk indexChunk =
-          newCreator.getChunk(CompactingMemStore.IndexType.CHUNK_MAP, ChunkType.INDEX_CHUNK);
+          newCreator.getChunk(ChunkType.INDEX_CHUNK);
       assertEquals(0, newCreator.getPoolSize());
       assertEquals(initialCount - 1, newCreator.getPoolSize(ChunkType.INDEX_CHUNK));
       assertTrue(indexChunk.isIndexChunk());
@@ -415,10 +415,10 @@ public void testNoIndexChunksPoolOrNoDataChunksPool() throws Exception {
     // Test both dataChunksPool and indexChunksPool are not null
     assertTrue(ChunkCreator.getInstance().getDataChunksPool() != null);
     assertTrue(ChunkCreator.getInstance().getIndexChunksPool() != null);
-    Chunk dataChunk = ChunkCreator.getInstance().getChunk(CompactingMemStore.IndexType.CHUNK_MAP);
+    Chunk dataChunk = ChunkCreator.getInstance().getChunk();
     assertTrue(dataChunk.isDataChunk());
     assertTrue(dataChunk.isFromPool());
-    Chunk indexChunk = ChunkCreator.getInstance().getChunk(CompactingMemStore.IndexType.CHUNK_MAP,
+    Chunk indexChunk = ChunkCreator.getInstance().getChunk(
       ChunkType.INDEX_CHUNK);
     assertTrue(indexChunk.isIndexChunk());
     assertTrue(indexChunk.isFromPool());

hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStoreLAB.java

@@ -39,6 +39,7 @@
 import org.apache.hadoop.hbase.MultithreadedTestUtil;
 import org.apache.hadoop.hbase.MultithreadedTestUtil.TestThread;
 import org.apache.hadoop.hbase.io.util.MemorySizeUtil;
+import org.apache.hadoop.hbase.regionserver.ChunkCreator.ChunkType;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
 import org.apache.hadoop.hbase.testclassification.RegionServerTests;
 import org.apache.hadoop.hbase.util.Bytes;
@@ -298,6 +299,34 @@ public void testForceCopyOfBigCellInto() {
       .currentTime(), bigValue);
     assertEquals(bigKV.getSerializedSize(),
       mslab.forceCopyOfBigCellInto(bigKV).getSerializedSize());
+
+    /**
+     * Add test by HBASE-26576,to test all the chunks are in {@link ChunkCreator#chunkIdMap}
+     */
+    assertTrue(mslab.chunks.size() == 2);
+    Chunk dataChunk = null;
+    Chunk jumboChunk = null;
+
+    for (Integer chunkId : mslab.chunks) {
+      Chunk chunk = ChunkCreator.getInstance().getChunk(chunkId);
+      assertTrue(chunk != null);
+      if (chunk.getChunkType() == ChunkType.JUMBO_CHUNK) {
+        jumboChunk = chunk;
+      } else if (chunk.getChunkType() == ChunkType.DATA_CHUNK) {
+        dataChunk = chunk;
+      }
+    }
+
+    assertTrue(dataChunk != null);
+    assertTrue(jumboChunk != null);
+
+    mslab.close();
+    /**
+     * After mslab close,jumboChunk was removed but dataChunk is recycled to pool
+     */
+    assertTrue(ChunkCreator.getInstance().getChunk(jumboChunk.getId()) == null);
+    assertTrue(ChunkCreator.getInstance().getChunk(dataChunk.getId()) == dataChunk);
+
   }
 
   private Thread getChunkQueueTestThread(final MemStoreLABImpl mslab, String threadName,