This is an automated email from the ASF dual-hosted git repository.
bbeaudreault pushed a commit to branch branch-2.5
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2.5 by this push:
new 25136c39f98 HBASE-27229 BucketCache statistics should not count
evictions by hfile (#4639)
25136c39f98 is described below
commit 25136c39f983b1ebb50e874591021338c23a2d7e
Author: Bryan Beaudreault <[email protected]>
AuthorDate: Tue Jul 26 16:59:57 2022 -0400
HBASE-27229 BucketCache statistics should not count evictions by hfile
(#4639)
Signed-off-by: Andrew Purtell <[email protected]>
---
.../hadoop/hbase/io/hfile/bucket/BucketCache.java | 24 +++++-----
.../hbase/io/hfile/bucket/TestBucketCache.java | 52 +++++++++++++++++++++-
.../io/hfile/bucket/TestBucketCacheRefCnt.java | 12 ++---
3 files changed, 71 insertions(+), 17 deletions(-)
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java
index 8f4b1830554..73f2bc71c31 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java
@@ -555,13 +555,16 @@ public class BucketCache implements BlockCache, HeapSize {
/**
* This method is invoked after the bucketEntry is removed from {@link
BucketCache#backingMap}
*/
- void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry, boolean
decrementBlockNumber) {
+ void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry, boolean
decrementBlockNumber,
+ boolean evictedByEvictionProcess) {
bucketEntry.markAsEvicted();
blocksByHFile.remove(cacheKey);
if (decrementBlockNumber) {
this.blockNumber.decrement();
}
- cacheStats.evicted(bucketEntry.getCachedTime(), cacheKey.isPrimary());
+ if (evictedByEvictionProcess) {
+ cacheStats.evicted(bucketEntry.getCachedTime(), cacheKey.isPrimary());
+ }
}
/**
@@ -591,7 +594,7 @@ public class BucketCache implements BlockCache, HeapSize {
*/
@Override
public boolean evictBlock(BlockCacheKey cacheKey) {
- return doEvictBlock(cacheKey, null);
+ return doEvictBlock(cacheKey, null, false);
}
/**
@@ -603,7 +606,8 @@ public class BucketCache implements BlockCache, HeapSize {
* @param bucketEntry {@link BucketEntry} matched {@link BlockCacheKey} to
evict.
* @return true to indicate whether we've evicted successfully or not.
*/
- private boolean doEvictBlock(BlockCacheKey cacheKey, BucketEntry
bucketEntry) {
+ private boolean doEvictBlock(BlockCacheKey cacheKey, BucketEntry bucketEntry,
+ boolean evictedByEvictionProcess) {
if (!cacheEnabled) {
return false;
}
@@ -614,14 +618,14 @@ public class BucketCache implements BlockCache, HeapSize {
final BucketEntry bucketEntryToUse = bucketEntry;
if (bucketEntryToUse == null) {
- if (existedInRamCache) {
+ if (existedInRamCache && evictedByEvictionProcess) {
cacheStats.evicted(0, cacheKey.isPrimary());
}
return existedInRamCache;
} else {
return bucketEntryToUse.withWriteLock(offsetLock, () -> {
if (backingMap.remove(cacheKey, bucketEntryToUse)) {
- blockEvicted(cacheKey, bucketEntryToUse, !existedInRamCache);
+ blockEvicted(cacheKey, bucketEntryToUse, !existedInRamCache,
evictedByEvictionProcess);
return true;
}
return false;
@@ -671,7 +675,7 @@ public class BucketCache implements BlockCache, HeapSize {
*/
boolean evictBucketEntryIfNoRpcReferenced(BlockCacheKey blockCacheKey,
BucketEntry bucketEntry) {
if (!bucketEntry.isRpcRef()) {
- return doEvictBlock(blockCacheKey, bucketEntry);
+ return doEvictBlock(blockCacheKey, bucketEntry, true);
}
return false;
}
@@ -792,7 +796,7 @@ public class BucketCache implements BlockCache, HeapSize {
* blocks evicted
* @param why Why we are being called
*/
- private void freeSpace(final String why) {
+ void freeSpace(final String why) {
// Ensure only one freeSpace progress at a time
if (!freeSpaceLock.tryLock()) {
return;
@@ -986,7 +990,7 @@ public class BucketCache implements BlockCache, HeapSize {
BucketEntry previousEntry = backingMap.put(key, bucketEntry);
if (previousEntry != null && previousEntry != bucketEntry) {
previousEntry.withWriteLock(offsetLock, () -> {
- blockEvicted(key, previousEntry, false);
+ blockEvicted(key, previousEntry, false, false);
return null;
});
}
@@ -1137,7 +1141,7 @@ public class BucketCache implements BlockCache, HeapSize {
final BucketEntry bucketEntry = bucketEntries[i];
bucketEntry.withWriteLock(offsetLock, () -> {
if (backingMap.remove(key, bucketEntry)) {
- blockEvicted(key, bucketEntry, false);
+ blockEvicted(key, bucketEntry, false, false);
}
return null;
});
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java
index 2ddc31df61f..6f7d4e22e2b 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java
@@ -263,7 +263,7 @@ public class TestBucketCache {
};
evictThread.start();
cache.offsetLock.waitForWaiters(lockId, 1);
- cache.blockEvicted(cacheKey, cache.backingMap.remove(cacheKey), true);
+ cache.blockEvicted(cacheKey, cache.backingMap.remove(cacheKey), true,
true);
assertEquals(0, cache.getBlockCount());
cacheAndWaitUntilFlushedToBucket(cache, cacheKey,
new CacheTestUtils.ByteArrayCacheable(new byte[10]));
@@ -481,6 +481,56 @@ public class TestBucketCache {
assertEquals(testValue, bucketEntry.offset());
}
+ @Test
+ public void testEvictionCount() throws InterruptedException {
+ int size = 100;
+ int length = HConstants.HFILEBLOCK_HEADER_SIZE + size;
+ ByteBuffer buf1 = ByteBuffer.allocate(size), buf2 =
ByteBuffer.allocate(size);
+ HFileContext meta = new HFileContextBuilder().build();
+ ByteBuffAllocator allocator = ByteBuffAllocator.HEAP;
+ HFileBlock blockWithNextBlockMetadata = new HFileBlock(BlockType.DATA,
size, size, -1,
+ ByteBuff.wrap(buf1), HFileBlock.FILL_HEADER, -1, 52, -1, meta,
allocator);
+ HFileBlock blockWithoutNextBlockMetadata = new HFileBlock(BlockType.DATA,
size, size, -1,
+ ByteBuff.wrap(buf2), HFileBlock.FILL_HEADER, -1, -1, -1, meta,
allocator);
+
+ BlockCacheKey key = new BlockCacheKey("testEvictionCount", 0);
+ ByteBuffer actualBuffer = ByteBuffer.allocate(length);
+ ByteBuffer block1Buffer = ByteBuffer.allocate(length);
+ ByteBuffer block2Buffer = ByteBuffer.allocate(length);
+ blockWithNextBlockMetadata.serialize(block1Buffer, true);
+ blockWithoutNextBlockMetadata.serialize(block2Buffer, true);
+
+ // Add blockWithNextBlockMetadata, expect blockWithNextBlockMetadata back.
+ CacheTestUtils.getBlockAndAssertEquals(cache, key,
blockWithNextBlockMetadata, actualBuffer,
+ block1Buffer);
+
+ waitUntilFlushedToBucket(cache, key);
+
+ assertEquals(0, cache.getStats().getEvictionCount());
+
+ // evict call should return 1, but then eviction count be 0
+ assertEquals(1, cache.evictBlocksByHfileName("testEvictionCount"));
+ assertEquals(0, cache.getStats().getEvictionCount());
+
+ // add back
+ CacheTestUtils.getBlockAndAssertEquals(cache, key,
blockWithNextBlockMetadata, actualBuffer,
+ block1Buffer);
+ waitUntilFlushedToBucket(cache, key);
+
+ // should not increment
+ assertTrue(cache.evictBlock(key));
+ assertEquals(0, cache.getStats().getEvictionCount());
+
+ // add back
+ CacheTestUtils.getBlockAndAssertEquals(cache, key,
blockWithNextBlockMetadata, actualBuffer,
+ block1Buffer);
+ waitUntilFlushedToBucket(cache, key);
+
+ // should finally increment eviction count
+ cache.freeSpace("testing");
+ assertEquals(1, cache.getStats().getEvictionCount());
+ }
+
@Test
public void testCacheBlockNextBlockMetadataMissing() throws Exception {
int size = 100;
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCacheRefCnt.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCacheRefCnt.java
index 13f1015954d..44a398bda44 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCacheRefCnt.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCacheRefCnt.java
@@ -557,10 +557,10 @@ public class TestBucketCacheRefCnt {
}
@Override
- void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry,
- boolean decrementBlockNumber) {
+ void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry, boolean
decrementBlockNumber,
+ boolean evictedByEvictionProcess) {
blockEvictCounter.incrementAndGet();
- super.blockEvicted(cacheKey, bucketEntry, decrementBlockNumber);
+ super.blockEvicted(cacheKey, bucketEntry, decrementBlockNumber,
evictedByEvictionProcess);
}
/**
@@ -709,8 +709,8 @@ public class TestBucketCacheRefCnt {
}
@Override
- void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry,
- boolean decrementBlockNumber) {
+ void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry, boolean
decrementBlockNumber,
+ boolean evictedByEvictionProcess) {
/**
* This is only invoked by {@link BucketCache.WriterThread}. {@link
MyMyBucketCache2} create
* only one {@link BucketCache.WriterThread}.
@@ -718,7 +718,7 @@ public class TestBucketCacheRefCnt {
assertTrue(Thread.currentThread() == this.writerThreads[0]);
blockEvictCounter.incrementAndGet();
- super.blockEvicted(cacheKey, bucketEntry, decrementBlockNumber);
+ super.blockEvicted(cacheKey, bucketEntry, decrementBlockNumber,
evictedByEvictionProcess);
}
/**
