openinx commented on a change in pull request #479: HBASE-22802 Avoid temp
ByteBuffer allocation in FileIOEngine#read
URL: https://github.com/apache/hbase/pull/479#discussion_r313260023
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java
##########
@@ -502,8 +502,10 @@ public Cacheable getBlock(BlockCacheKey key, boolean
caching, boolean repeat,
// block will use the refCnt of bucketEntry, which means if two
HFileBlock mapping to
// the same BucketEntry, then all of the three will share the same
refCnt.
Cacheable cachedBlock = ioEngine.read(bucketEntry);
- // RPC start to reference, so retain here.
- cachedBlock.retain();
+ if (ioEngine.usesSharedMemory()) {
Review comment:
Could we consider the patch in another side ? Say the BucketEntry#refCnt
still means how many reference path from the RPC handler & BucketCache ... But
the Cacheable#refCnt will always be 1 when the RPC is still handling.. Once the
cells shipped to client, then both the Cacheable#refCnt & BucketEntry#refCnt
will decrease, actually the Cacheable will de-allocate the memory from
ByteBuffAllocator.
In this way, the BucketEntry#refCnt still have the same meaning as the
SharedFileIOEngine in BucketCache. I mean in BucketCache we don't need to care
the IOEngine is shared or exclusive.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
