saintstack commented on a change in pull request #3215:
URL: https://github.com/apache/hbase/pull/3215#discussion_r638134220
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/TinyLfuBlockCache.java
##########
@@ -158,7 +158,13 @@ public boolean containsBlock(BlockCacheKey cacheKey) {
@Override
public Cacheable getBlock(BlockCacheKey cacheKey,
boolean caching, boolean repeat, boolean updateCacheMetrics) {
- Cacheable value = cache.getIfPresent(cacheKey);
+ Cacheable value = cache.asMap().computeIfPresent(cacheKey, (blockCacheKey,
cacheable) -> {
+ // It will be referenced by RPC path, so increase here. NOTICE: Must do
the retain inside
+ // this block. because if retain outside the map#computeIfPresent, the
evictBlock may remove
+ // the block and release, then we're retaining a block with refCnt=0
which is disallowed.
+ cacheable.retain();
+ return cacheable;
+ });
Review comment:
Thanks @virajjasani ... I just read over the HBASE-22422. Nice work by
@openinx . Agree need locking but currently the lock spans more than the
buffer... covering CHM bucket. We might be able to scope the lock to the
buffer... but I'm not sure and a little afraid to touch the code here.
On the sawtooth, I've not looked.
Not using offheap. All onheap but in async-profiler, the
CHM#computeIfPresent is the main blocking point by far (Trying to drive up the
throughput when inmemory meta lookups).
On this patch generally, +1. I'd noticed messing w/ this stuff that tinylfu
was missing this bit of code... I'd also noticed that the locking profile with
tinylfu in place looked much better .... I'd failed to put the two bits of
info together. My sense is that once this goes in, that tinylfu will be the
main blocker... just like CHM is now.
Oddly, for me, tinylfu seemed to run slower in my compares which I didn't
expect given the nice batching it does and its WAL scheme. Its probably
variance in my test rig. Working on 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.
For queries about this service, please contact Infrastructure at:
[email protected]
