[ 
https://issues.apache.org/jira/browse/HBASE-11520?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14062926#comment-14062926
 ] 

stack commented on HBASE-11520:
-------------------------------

Nice review.

Thank you for prompting the refactor.  It was hard to grok as it was.  It is a 
bit cleaner now (could be refactored more but will leave as is else will start 
to ripple afar).

On the test, offheap plus BUCKET_CACHE_COMBINED_KEY == false is a legit set up. 
 This is how you ask for an L1+L2 that is not 'combined'.  The L2 gets 
registered as the victimHandler for L1.  When eviction thread runs, blocks 
moved to L2.  When we search for blocks, we'll look in both places (first in 
LruBC then in its victim if not present...).  Given this latter, the 
package-info as it is in the patch is right by my reckoning.  From 
LruBlockCache:

{code}
  public Cacheable getBlock(BlockCacheKey cacheKey, boolean caching, boolean 
repeat,
      boolean updateCacheMetrics) {
    LruCachedBlock cb = map.get(cacheKey);
    if (cb == null) {
      if (!repeat && updateCacheMetrics) stats.miss(caching);
      if (victimHandler != null) {
        return victimHandler.getBlock(cacheKey, caching, repeat, 
updateCacheMetrics);
      }
      return null;
    }
    if (updateCacheMetrics) stats.hit(caching);
    cb.access(count.incrementAndGet());
    return cb.getBuffer();
  }
{code}



> Simplify offheap cache config by removing the confusing 
> "hbase.bucketcache.percentage.in.combinedcache"
> -------------------------------------------------------------------------------------------------------
>
>                 Key: HBASE-11520
>                 URL: https://issues.apache.org/jira/browse/HBASE-11520
>             Project: HBase
>          Issue Type: Sub-task
>          Components: io
>    Affects Versions: 0.99.0
>            Reporter: stack
>            Assignee: stack
>             Fix For: 0.99.0, 2.0.0
>
>         Attachments: 11520.txt, 11520v2.txt
>
>
> Remove "hbase.bucketcache.percentage.in.combinedcache".  It is unnecessary 
> complication of block cache config.  Let L1 config setup be as it is whether 
> a L2 present or not, just set hfile.block.cache.size (not 
> hbase.bucketcache.size * (1.0 - 
> hbase.bucketcache.percentage.in.combinedcache)).  For L2, let 
> hbase.bucketcache.size be the actual size of the bucket cache, not 
> hbase.bucketcache.size * hbase.bucketcache.percentage.in.combinedcache.
> Attached patch removes the config. and updates docs.  Adds tests to confirm 
> configs are as expected whether a CombinedBlockCache deploy or a strict L1+L2 
> deploy.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to