[jira] [Resolved] (HBASE-28458) BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached

["Wellington Chevreuil (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/HBASE-28458?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Wellington Chevreuil resolved HBASE-28458.
------------------------------------------
    Resolution: Fixed

Merged into master, branch-3, branch-2 and branch-2.6. Thanks for reviewing it [~zhangduo] [~psomogyi] !

> BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached
> -----------------------------------------------------------------------------------
>
>                 Key: HBASE-28458
>                 URL: https://issues.apache.org/jira/browse/HBASE-28458
>             Project: HBase
>          Issue Type: Bug
>    Affects Versions: 2.6.0, 3.0.0-beta-1, 4.0.0-alpha-1, 2.7.0
>            Reporter: Wellington Chevreuil
>            Assignee: Wellington Chevreuil
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 2.6.0, 3.0.0, 4.0.0-alpha-1, 2.7.0
>
>
> Noticed that TestBucketCachePersister.testPrefetchBlockEvictionWhilePrefetchRunning was flakey, failing whenever the block eviction happened while prefetch was still ongoing.
> In the test, we pass an instance of BucketCache directly to the cache config, so the test is actually placing both data and meta blocks in the bucket cache. So sometimes, the test call BucketCache.notifyFileCachingCompleted after the it has already evicted two blocks.  
> Inside BucketCache.notifyFileCachingCompleted, we iterate through the backingMap entry set, counting number of blocks for the given file. Then, to consider whether the file is fully cached or not, we do the following validation:
> {noformat}
> if (dataBlockCount == count.getValue() || totalBlockCount == count.getValue()) {
>   LOG.debug("File {} has now been fully cached.", fileName);
>   fileCacheCompleted(fileName, size);
> }  {noformat}
> But the test generates 57 total blocks, 55 data and 2 meta blocks. It evicts two blocks and asserts that the file hasn't been considered fully cached. When these evictions happen while prefetch is still going, we'll pass that check, as the the number of blocks for the file in the backingMap would still be 55, which is what we pass as dataBlockCount.
> As BucketCache is intended for storing data blocks only, I believe we should make sure BucketCache.notifyFileCachingCompleted only accounts for data blocks. Also, the TestBucketCachePersister.testPrefetchBlockEvictionWhilePrefetchRunning should be updated to consistently reproduce the eviction concurrent to the prefetch. 
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)