Vladimir Rodionov created HBASE-30412:
-----------------------------------------
Summary: Fix block replacement accounting and reference handling
in LruBlockCache/LruCacheEngine
Key: HBASE-30412
URL: https://issues.apache.org/jira/browse/HBASE-30412
Project: HBase
Issue Type: New Feature
Components: BlockCache
Affects Versions: 2.6.7, 2.5.16, 3.0.0, 4.0.0-alpha-1
Reporter: Vladimir Rodionov
Assignee: Vladimir Rodionov
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)
