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

Anoop Sam John commented on HBASE-16650:
----------------------------------------

bq.We are setting some flags in this block that are used later. Do we need 
those flags when an eviction because an hfile has been removed?
No there are no flags being set inside the if and used outside..  
Code before patch
{code}
stats.evicted(block.getCachedTime(), block.getCacheKey().isPrimary());
    if (evictedByEvictionProcess && victimHandler != null) {
      if (victimHandler instanceof BucketCache) {
        boolean wait = getCurrentSize() < acceptableSize();
        boolean inMemory = block.getPriority() == BlockPriority.MEMORY;
        ((BucketCache)victimHandler).cacheBlockWithWait(block.getCacheKey(), 
block.getBuffer(),
            inMemory, wait);
      } else {
        victimHandler.cacheBlock(block.getCacheKey(), block.getBuffer());
      }
    }
{code}
Code after this patch
{code}
 if (evictedByEvictionProcess ) {
   stats.evicted(block.getCachedTime(), block.getCacheKey().isPrimary());
     if(victimHandler != null){
      if (victimHandler instanceof BucketCache) {
        boolean wait = getCurrentSize() < acceptableSize();
        boolean inMemory = block.getPriority() == BlockPriority.MEMORY;
        ((BucketCache)victimHandler).cacheBlockWithWait(block.getCacheKey(), 
block.getBuffer(),
            inMemory, wait);
      } else {
        victimHandler.cacheBlock(block.getCacheKey(), block.getBuffer());
      }
   }
 }
{code}

Will commit with addressing ur 1st comment

> Wrong usage of BlockCache eviction stat for heap memory tuning
> --------------------------------------------------------------
>
>                 Key: HBASE-16650
>                 URL: https://issues.apache.org/jira/browse/HBASE-16650
>             Project: HBase
>          Issue Type: Sub-task
>    Affects Versions: 1.0.0
>            Reporter: Anoop Sam John
>            Assignee: Anoop Sam John
>             Fix For: 2.0.0, 1.4.0
>
>         Attachments: HBASE-16650.patch
>
>
> 1. We use the stat evictedBlocksCount - A block can get evicted because of 
> eviction thread due to lack of space or because of removal of an HFile itself 
> (After a compaction). We should not consider the latter in the tune decision 
> at all. These are actually invalidation of blocks. Should the stat counter 
> itself not use this count of evicted blocks? I think yes. This will give 
> wrong message to users that there are lot of real eviction happening.
> 2. In case L1+ L2 combined block cache, what we use is the sum of evictions 
> from both. But we will be tuning L1 size alone. Eviction count from L2 should 
> not affect the tuning of L1



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to