Skip to content

Commit 277058e

Browse files
bbeaudreaultbbeaudreault
authored
HBASE-27229 BucketCache statistics should not count evictions by hfile (#4639)
Signed-off-by: Andrew Purtell <apurtell@apache.org>
1 parent 8b091c4 commit 277058e

3 files changed

Lines changed: 71 additions & 17 deletions

File tree

‎hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -566,13 +566,16 @@ public Cacheable getBlock(BlockCacheKey key, boolean caching, boolean repeat,
566566
/**
567567
* This method is invoked after the bucketEntry is removed from {@link BucketCache#backingMap}
568568
*/
569-
void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry, boolean decrementBlockNumber) {
569+
void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry, boolean decrementBlockNumber,
570+
boolean evictedByEvictionProcess) {
570571
bucketEntry.markAsEvicted();
571572
blocksByHFile.remove(cacheKey);
572573
if (decrementBlockNumber) {
573574
this.blockNumber.decrement();
574575
}
575-
cacheStats.evicted(bucketEntry.getCachedTime(), cacheKey.isPrimary());
576+
if (evictedByEvictionProcess) {
577+
cacheStats.evicted(bucketEntry.getCachedTime(), cacheKey.isPrimary());
578+
}
576579
}
577580

578581
/**
@@ -602,7 +605,7 @@ void freeBucketEntry(BucketEntry bucketEntry) {
602605
*/
603606
@Override
604607
public boolean evictBlock(BlockCacheKey cacheKey) {
605-
return doEvictBlock(cacheKey, null);
608+
return doEvictBlock(cacheKey, null, false);
606609
}
607610

608611
/**
@@ -614,7 +617,8 @@ public boolean evictBlock(BlockCacheKey cacheKey) {
614617
* @param bucketEntry {@link BucketEntry} matched {@link BlockCacheKey} to evict.
615618
* @return true to indicate whether we've evicted successfully or not.
616619
*/
617-
private boolean doEvictBlock(BlockCacheKey cacheKey, BucketEntry bucketEntry) {
620+
private boolean doEvictBlock(BlockCacheKey cacheKey, BucketEntry bucketEntry,
621+
boolean evictedByEvictionProcess) {
618622
if (!cacheEnabled) {
619623
return false;
620624
}
@@ -625,14 +629,14 @@ private boolean doEvictBlock(BlockCacheKey cacheKey, BucketEntry bucketEntry) {
625629
final BucketEntry bucketEntryToUse = bucketEntry;
626630

627631
if (bucketEntryToUse == null) {
628-
if (existedInRamCache) {
632+
if (existedInRamCache && evictedByEvictionProcess) {
629633
cacheStats.evicted(0, cacheKey.isPrimary());
630634
}
631635
return existedInRamCache;
632636
} else {
633637
return bucketEntryToUse.withWriteLock(offsetLock, () -> {
634638
if (backingMap.remove(cacheKey, bucketEntryToUse)) {
635-
blockEvicted(cacheKey, bucketEntryToUse, !existedInRamCache);
639+
blockEvicted(cacheKey, bucketEntryToUse, !existedInRamCache, evictedByEvictionProcess);
636640
return true;
637641
}
638642
return false;
@@ -682,7 +686,7 @@ public boolean evictBlockIfNoRpcReferenced(BlockCacheKey blockCacheKey) {
682686
*/
683687
boolean evictBucketEntryIfNoRpcReferenced(BlockCacheKey blockCacheKey, BucketEntry bucketEntry) {
684688
if (!bucketEntry.isRpcRef()) {
685-
return doEvictBlock(blockCacheKey, bucketEntry);
689+
return doEvictBlock(blockCacheKey, bucketEntry, true);
686690
}
687691
return false;
688692
}
@@ -803,7 +807,7 @@ private void freeEntireBuckets(int completelyFreeBucketsNeeded) {
803807
* blocks evicted
804808
* @param why Why we are being called
805809
*/
806-
private void freeSpace(final String why) {
810+
void freeSpace(final String why) {
807811
// Ensure only one freeSpace progress at a time
808812
if (!freeSpaceLock.tryLock()) {
809813
return;
@@ -997,7 +1001,7 @@ protected void putIntoBackingMap(BlockCacheKey key, BucketEntry bucketEntry) {
9971001
BucketEntry previousEntry = backingMap.put(key, bucketEntry);
9981002
if (previousEntry != null && previousEntry != bucketEntry) {
9991003
previousEntry.withWriteLock(offsetLock, () -> {
1000-
blockEvicted(key, previousEntry, false);
1004+
blockEvicted(key, previousEntry, false, false);
10011005
return null;
10021006
});
10031007
}
@@ -1148,7 +1152,7 @@ void doDrain(final List<RAMQueueEntry> entries, ByteBuffer metaBuff) throws Inte
11481152
final BucketEntry bucketEntry = bucketEntries[i];
11491153
bucketEntry.withWriteLock(offsetLock, () -> {
11501154
if (backingMap.remove(key, bucketEntry)) {
1151-
blockEvicted(key, bucketEntry, false);
1155+
blockEvicted(key, bucketEntry, false, false);
11521156
}
11531157
return null;
11541158
});

‎hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java‎

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ public void run() {
262262
};
263263
evictThread.start();
264264
cache.offsetLock.waitForWaiters(lockId, 1);
265-
cache.blockEvicted(cacheKey, cache.backingMap.remove(cacheKey), true);
265+
cache.blockEvicted(cacheKey, cache.backingMap.remove(cacheKey), true, true);
266266
assertEquals(0, cache.getBlockCount());
267267
cacheAndWaitUntilFlushedToBucket(cache, cacheKey,
268268
new CacheTestUtils.ByteArrayCacheable(new byte[10]));
@@ -576,6 +576,56 @@ public void testOffsetProducesPositiveOutput() {
576576
assertEquals(testValue, bucketEntry.offset());
577577
}
578578

579+
@Test
580+
public void testEvictionCount() throws InterruptedException {
581+
int size = 100;
582+
int length = HConstants.HFILEBLOCK_HEADER_SIZE + size;
583+
ByteBuffer buf1 = ByteBuffer.allocate(size), buf2 = ByteBuffer.allocate(size);
584+
HFileContext meta = new HFileContextBuilder().build();
585+
ByteBuffAllocator allocator = ByteBuffAllocator.HEAP;
586+
HFileBlock blockWithNextBlockMetadata = new HFileBlock(BlockType.DATA, size, size, -1,
587+
ByteBuff.wrap(buf1), HFileBlock.FILL_HEADER, -1, 52, -1, meta, allocator);
588+
HFileBlock blockWithoutNextBlockMetadata = new HFileBlock(BlockType.DATA, size, size, -1,
589+
ByteBuff.wrap(buf2), HFileBlock.FILL_HEADER, -1, -1, -1, meta, allocator);
590+
591+
BlockCacheKey key = new BlockCacheKey("testEvictionCount", 0);
592+
ByteBuffer actualBuffer = ByteBuffer.allocate(length);
593+
ByteBuffer block1Buffer = ByteBuffer.allocate(length);
594+
ByteBuffer block2Buffer = ByteBuffer.allocate(length);
595+
blockWithNextBlockMetadata.serialize(block1Buffer, true);
596+
blockWithoutNextBlockMetadata.serialize(block2Buffer, true);
597+
598+
// Add blockWithNextBlockMetadata, expect blockWithNextBlockMetadata back.
599+
CacheTestUtils.getBlockAndAssertEquals(cache, key, blockWithNextBlockMetadata, actualBuffer,
600+
block1Buffer);
601+
602+
waitUntilFlushedToBucket(cache, key);
603+
604+
assertEquals(0, cache.getStats().getEvictionCount());
605+
606+
// evict call should return 1, but then eviction count be 0
607+
assertEquals(1, cache.evictBlocksByHfileName("testEvictionCount"));
608+
assertEquals(0, cache.getStats().getEvictionCount());
609+
610+
// add back
611+
CacheTestUtils.getBlockAndAssertEquals(cache, key, blockWithNextBlockMetadata, actualBuffer,
612+
block1Buffer);
613+
waitUntilFlushedToBucket(cache, key);
614+
615+
// should not increment
616+
assertTrue(cache.evictBlock(key));
617+
assertEquals(0, cache.getStats().getEvictionCount());
618+
619+
// add back
620+
CacheTestUtils.getBlockAndAssertEquals(cache, key, blockWithNextBlockMetadata, actualBuffer,
621+
block1Buffer);
622+
waitUntilFlushedToBucket(cache, key);
623+
624+
// should finally increment eviction count
625+
cache.freeSpace("testing");
626+
assertEquals(1, cache.getStats().getEvictionCount());
627+
}
628+
579629
@Test
580630
public void testCacheBlockNextBlockMetadataMissing() throws Exception {
581631
int size = 100;

‎hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCacheRefCnt.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -557,10 +557,10 @@ protected void cacheBlockWithWaitInternal(BlockCacheKey cacheKey, Cacheable cach
557557
}
558558

559559
@Override
560-
void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry,
561-
boolean decrementBlockNumber) {
560+
void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry, boolean decrementBlockNumber,
561+
boolean evictedByEvictionProcess) {
562562
blockEvictCounter.incrementAndGet();
563-
super.blockEvicted(cacheKey, bucketEntry, decrementBlockNumber);
563+
super.blockEvicted(cacheKey, bucketEntry, decrementBlockNumber, evictedByEvictionProcess);
564564
}
565565

566566
/**
@@ -709,16 +709,16 @@ protected boolean removeFromRamCache(BlockCacheKey cacheKey) {
709709
}
710710

711711
@Override
712-
void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry,
713-
boolean decrementBlockNumber) {
712+
void blockEvicted(BlockCacheKey cacheKey, BucketEntry bucketEntry, boolean decrementBlockNumber,
713+
boolean evictedByEvictionProcess) {
714714
/**
715715
* This is only invoked by {@link BucketCache.WriterThread}. {@link MyMyBucketCache2} create
716716
* only one {@link BucketCache.WriterThread}.
717717
*/
718718
assertTrue(Thread.currentThread() == this.writerThreads[0]);
719719

720720
blockEvictCounter.incrementAndGet();
721-
super.blockEvicted(cacheKey, bucketEntry, decrementBlockNumber);
721+
super.blockEvicted(cacheKey, bucketEntry, decrementBlockNumber, evictedByEvictionProcess);
722722
}
723723

724724
/**

0 commit comments

Comments
 (0)