comnetwork commented on a change in pull request #3680:
URL: https://github.com/apache/hbase/pull/3680#discussion_r708213574
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java
##########
@@ -906,134 +968,135 @@ public void run() {
}
LOG.info(this.getName() + " exiting, cacheEnabled=" + cacheEnabled);
}
+ }
- /**
- * Put the new bucket entry into backingMap. Notice that we are allowed to
replace the existing
- * cache with a new block for the same cache key. there's a corner case:
one thread cache a
- * block in ramCache, copy to io-engine and add a bucket entry to
backingMap. Caching another
- * new block with the same cache key do the same thing for the same cache
key, so if not evict
- * the previous bucket entry, then memory leak happen because the previous
bucketEntry is gone
- * but the bucketAllocator do not free its memory.
- * @see BlockCacheUtil#shouldReplaceExistingCacheBlock(BlockCache
blockCache,BlockCacheKey
- * cacheKey, Cacheable newBlock)
- * @param key Block cache key
- * @param bucketEntry Bucket entry to put into backingMap.
- */
- private void putIntoBackingMap(BlockCacheKey key, BucketEntry bucketEntry)
{
- BucketEntry previousEntry = backingMap.put(key, bucketEntry);
- if (previousEntry != null && previousEntry != bucketEntry) {
- previousEntry.withWriteLock(offsetLock, () -> {
- blockEvicted(key, previousEntry, false);
- return null;
- });
- }
+ /**
Review comment:
Here I move `putIntoBackingMap `from `WriterThread `to `BucketCache `to
facilitate test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
