[GitHub] [hbase] anoopsjohn commented on a change in pull request #257: HBASE-22463 Some paths in HFileScannerImpl did not consider block#release which will exhaust the ByteBuffAllocator

[GitBox <gi...@apache.org>]

anoopsjohn commented on a change in pull request #257: HBASE-22463 Some paths in HFileScannerImpl did not consider block#release which will exhaust the ByteBuffAllocator
URL: https://github.com/apache/hbase/pull/257#discussion_r288383942
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderImpl.java
 ##########
 @@ -520,20 +520,18 @@ public HFileScannerImpl(final HFile.Reader reader, final boolean cacheBlocks,
     }
 
     void updateCurrBlockRef(HFileBlock block) {
-      if (block != null && this.curBlock != null &&
-          block.getOffset() == this.curBlock.getOffset()) {
+      if (block != null && curBlock != null && block.getOffset() == curBlock.getOffset()) {
         return;
       }
-      // We don't have to keep ref to EXCLUSIVE type of block
-      if (this.curBlock != null && this.curBlock.usesSharedMemory()) {
 
 Review comment:
   There is a  concern here. Even if the block is on an exclusive heap memory area, we will keep ref to that in this list.  In a Phoenix Aggregation kind of use case where many blocks might get fetched and not immediately shipped, we are keeping the ref unwantedly here for longer time. This makes the GC not able to reclaim the heap memory area for the blocks.  This might be a hidden bomb IMO.  Its not good to remove the MemType.  Lets create the block with memory type as EXCLUSIVE  when the block data is on heap. The block might be coming from LRU cache or by fetching the block data from HDFS into heap memory area.   When the block comes from off heap BC or if it is backed by a BB from the pool (While reading from HDFS, read into pooled BB) lets create the block with mem type as SHARED.   Every block can have the retain and release method but let the EXCLUSIVE types do a noop here.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services