[
https://issues.apache.org/jira/browse/HBASE-30412?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Vladimir Rodionov updated HBASE-30412:
--------------------------------------
Summary: Fix block replacement accounting and reference handling in
LruBlockCache (was: Fix block replacement accounting and reference handling in
LruBlockCache/LruCacheEngine)
> Fix block replacement accounting and reference handling in LruBlockCache
> ------------------------------------------------------------------------
>
> Key: HBASE-30412
> URL: https://issues.apache.org/jira/browse/HBASE-30412
> Project: HBase
> Issue Type: New Feature
> Components: BlockCache
> Affects Versions: 3.0.0, 4.0.0-alpha-1, 2.5.16, 2.6.7
> Reporter: Vladimir Rodionov
> Assignee: Vladimir Rodionov
> Priority: Major
> Fix For: 4.0.0-alpha-1, 3.0.1, 2.6.8, 2.5.17
>
>
> h2. Description
> {{LruBlockCache}} contain a block replacement path used when
> {{BlockCacheUtil.shouldReplaceExistingCacheBlock(...)}} allows an existing
> cached block to be replaced.
> The current sequence is approximately:
> {code:java}
> LruCachedBlock cachedBlock = map.get(cacheKey);
> if (
> cachedBlock != null &&
> !BlockCacheUtil.shouldReplaceExistingCacheBlock(this, cacheKey, buf)
> ) {
> return;
> }
> Cacheable referencedBlock = asReferencedHeapBlock(buf);
> cachedBlock = new LruCachedBlock(cacheKey, referencedBlock,
> count.incrementAndGet(), inMemory);
> long newSize = updateSizeMetrics(cachedBlock, false);
> map.put(cacheKey, cachedBlock);
> elements.incrementAndGet();
> {code}
> When replacement is allowed, {{map.put()}} overwrites the previous
> {{LruCachedBlock}}.
> The previous cached entry is not removed through the normal eviction/removal
> path before the new
> entry is accounted for. As a result, the old cache-owned buffer reference may
> not be released and
> the old block's size and block-count accounting may remain included in cache
> statistics.
> Repeated replacements can therefore cause cache occupancy and block counters
> to diverge from the
> actual contents of the backing map and may retain block references longer
> than intended.
> h2. Proposed Changes
> * Make replacement of an existing {{LruCachedBlock}} atomic with respect to
> the backing map.
> * Perform removal accounting against the actual entry being replaced.
> * Release the cache-owned reference held by the replaced block.
> * Subtract the replaced block's size from total and block-type-specific size
> counters.
> * Update element and block-type counters correctly for replacement rather
> than treating the
> operation as an additional insertion.
> * Preserve the behavior of
> {{BlockCacheUtil.shouldReplaceExistingCacheBlock(...)}}.
> * Preserve existing cache hit/miss and eviction statistics semantics.
> * Add tests covering repeated replacement of the same {{BlockCacheKey}}.
> h2. Concurrency Considerations
> The replacement operation should account for concurrent changes to the same
> cache key.
> The implementation should avoid performing accounting based on an entry
> obtained by an earlier
> {{map.get()}} if that entry is no longer the value being replaced.
> Removal/reference accounting must be applied to the actual previous mapping
> replaced by the
> operation.
> h2. Acceptance Criteria
> * Replacing an existing cached block releases the cache-owned reference of
> the replaced block.
> * Cache size accounting reflects only the currently cached replacement block.
> * {{elements}} and block-type element counters do not increase merely because
> an existing key is
> replaced.
> * Repeated replacements of the same cache key do not cause cache-size or
> block-count drift.
> * Replacement remains safe under concurrent access/replacement of the same
> cache key.
> * Existing HBASE-20447 replacement behavior is preserved.
> * Tests verify reference-count and cache-accounting behavior for block
> replacement.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)