[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4931: HBASE-27233 Read blocks into off-heap if caching is disabled for read

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

bbeaudreault commented on code in PR #4931:
URL: https://github.com/apache/hbase/pull/4931#discussion_r1053750466


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderImpl.java:
##########
@@ -1321,8 +1334,13 @@ public HFileBlock readBlock(long dataBlockOffset, long onDiskBlockSize, final bo
         span.addEvent("block cache miss", attributes);
         // Load block from filesystem.
         HFileBlock hfileBlock = fsBlockReader.readBlockData(dataBlockOffset, onDiskBlockSize, pread,
-          !isCompaction, shouldUseHeap(expectedBlockType));
-        validateBlockType(hfileBlock, expectedBlockType);
+          !isCompaction, shouldUseHeap(expectedBlockType, cacheable));
+        try {
+          validateBlockType(hfileBlock, expectedBlockType);
+        } catch (IOException e) {
+          hfileBlock.release();
+          throw e;
+        }

Review Comment:
   I noticed this potential leak because my test case was triggering this validation error. It seems we must not pass in invalid block types here often because I've never seen this leak in production, but it's best to close it.



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org