(hbase-site) branch asf-site updated: INFRA-10751 Empty commit
This is an automated email from the ASF dual-hosted git repository. git-site-role pushed a commit to branch asf-site in repository https://gitbox.apache.org/repos/asf/hbase-site.git The following commit(s) were added to refs/heads/asf-site by this push: new 1fc4c61a1aa INFRA-10751 Empty commit 1fc4c61a1aa is described below commit 1fc4c61a1aac6f623832016a1d0e1258f359e8ff Author: jenkins AuthorDate: Fri Apr 5 14:45:59 2024 + INFRA-10751 Empty commit
(hbase) 02/02: [ADDENDUM] HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777) (#5791)
This is an automated email from the ASF dual-hosted git repository. wchevreuil pushed a commit to branch branch-2.6 in repository https://gitbox.apache.org/repos/asf/hbase.git commit a56126c27664be095c03120a237957f91db7be3a Author: Wellington Ramos Chevreuil AuthorDate: Fri Apr 5 10:56:06 2024 +0100 [ADDENDUM] HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777) (#5791) Signed-off-by: Peter Somogyi (cherry picked from commit d7566abd5de915e8f55a4f1f1939f6be38891657) --- .../hadoop/hbase/io/hfile/bucket/BucketCache.java | 48 ++ .../io/hfile/bucket/TestPrefetchPersistence.java | 35 +++- 2 files changed, 37 insertions(+), 46 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 55743e861af..c8111522c65 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 @@ -700,7 +700,8 @@ public class BucketCache implements BlockCache, HeapSize { } else { return bucketEntryToUse.withWriteLock(offsetLock, () -> { if (backingMap.remove(cacheKey, bucketEntryToUse)) { - LOG.debug("removed key {} from back map in the evict process", cacheKey); + LOG.debug("removed key {} from back map with offset lock {} in the evict process", +cacheKey, bucketEntryToUse.offset()); blockEvicted(cacheKey, bucketEntryToUse, !existedInRamCache, evictedByEvictionProcess); return true; } @@ -1605,19 +1606,21 @@ public class BucketCache implements BlockCache, HeapSize { @Override public int evictBlocksByHfileName(String hfileName) { removeFileFromPrefetch(hfileName); -Set keySet = blocksByHFile.subSet(new BlockCacheKey(hfileName, Long.MIN_VALUE), - true, new BlockCacheKey(hfileName, Long.MAX_VALUE), true); - +Set keySet = getAllCacheKeysForFile(hfileName); int numEvicted = 0; for (BlockCacheKey key : keySet) { if (evictBlock(key)) { ++numEvicted; } } - return numEvicted; } + private Set getAllCacheKeysForFile(String hfileName) { +return blocksByHFile.subSet(new BlockCacheKey(hfileName, Long.MIN_VALUE), true, + new BlockCacheKey(hfileName, Long.MAX_VALUE), true); + } + /** * Used to group bucket entries into priority buckets. There will be a BucketEntryGroup for each * priority (single, multi, memory). Once bucketed, the eviction algorithm takes the appropriate @@ -2030,26 +2033,33 @@ public class BucketCache implements BlockCache, HeapSize { entry.getKey().getHfileName().equals(fileName.getName()) && entry.getKey().getBlockType().equals(BlockType.DATA) ) { - LOG.debug("found block for file {} in the backing map. Acquiring read lock for offset {}", -fileName.getName(), entry.getKey().getOffset()); - ReentrantReadWriteLock lock = offsetLock.getLock(entry.getKey().getOffset()); + long offsetToLock = entry.getValue().offset(); + LOG.debug("found block {} in the backing map. Acquiring read lock for offset {}", +entry.getKey(), offsetToLock); + ReentrantReadWriteLock lock = offsetLock.getLock(offsetToLock); lock.readLock().lock(); locks.add(lock); // rechecks the given key is still there (no eviction happened before the lock acquired) if (backingMap.containsKey(entry.getKey())) { count.increment(); + } else { +lock.readLock().unlock(); +locks.remove(lock); +LOG.debug("found block {}, but when locked and tried to count, it was gone."); } } }); + int metaCount = totalBlockCount - dataBlockCount; // BucketCache would only have data blocks if (dataBlockCount == count.getValue()) { LOG.debug("File {} has now been fully cached.", fileName); fileCacheCompleted(fileName, size); } else { LOG.debug( - "Prefetch executor completed for {}, but only {} blocks were cached. " -+ "Total blocks for file: {}. Checking for blocks pending cache in cache writer queue.", - fileName.getName(), count.getValue(), dataBlockCount); + "Prefetch executor completed for {}, but only {} data blocks were cached. " ++ "Total data blocks for file: {}. " ++ "Checking for blocks pending cache in cache writer queue.", + fileName, count.getValue(), dataBlockCount); if (ramCache.hasBlocksForFile(fileName.getName())) { for (ReentrantReadWriteLock lock : locks) { lock.readLock().unlock(); @@ -2058,11 +2068,17 @@ public class
(hbase) 01/02: HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777)
This is an automated email from the ASF dual-hosted git repository. wchevreuil pushed a commit to branch branch-2.6 in repository https://gitbox.apache.org/repos/asf/hbase.git commit ee4b44fa504943c08722393c5847afbbb5877cc1 Author: Wellington Ramos Chevreuil AuthorDate: Tue Apr 2 11:58:53 2024 +0100 HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777) Signed-off-by: Duo Zhang (cherry picked from commit c4ac2df041aa4795f91024b1e5dc8d4f5b6c048e) --- .../hadoop/hbase/io/hfile/bucket/BucketCache.java | 26 +- .../hfile/TestBlockEvictionOnRegionMovement.java | 9 .../io/hfile/bucket/TestBucketCachePersister.java | 25 +++-- 3 files changed, 34 insertions(+), 26 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 965b9776023..55743e861af 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 @@ -2020,26 +2020,30 @@ public class BucketCache implements BlockCache, HeapSize { // so we need to count all blocks for this file in the backing map under // a read lock for the block offset final List locks = new ArrayList<>(); -LOG.debug("Notifying caching completed for file {}, with total blocks {}", fileName, - dataBlockCount); +LOG.debug("Notifying caching completed for file {}, with total blocks {}, and data blocks {}", + fileName, totalBlockCount, dataBlockCount); try { final MutableInt count = new MutableInt(); LOG.debug("iterating over {} entries in the backing map", backingMap.size()); backingMap.entrySet().stream().forEach(entry -> { -if (entry.getKey().getHfileName().equals(fileName.getName())) { +if ( + entry.getKey().getHfileName().equals(fileName.getName()) +&& entry.getKey().getBlockType().equals(BlockType.DATA) +) { LOG.debug("found block for file {} in the backing map. Acquiring read lock for offset {}", fileName.getName(), entry.getKey().getOffset()); ReentrantReadWriteLock lock = offsetLock.getLock(entry.getKey().getOffset()); lock.readLock().lock(); locks.add(lock); + // rechecks the given key is still there (no eviction happened before the lock acquired) if (backingMap.containsKey(entry.getKey())) { count.increment(); } } }); - // We may either place only data blocks on the BucketCache or all type of blocks - if (dataBlockCount == count.getValue() || totalBlockCount == count.getValue()) { -LOG.debug("File {} has now been fully cached.", fileName.getName()); + // BucketCache would only have data blocks + if (dataBlockCount == count.getValue()) { +LOG.debug("File {} has now been fully cached.", fileName); fileCacheCompleted(fileName, size); } else { LOG.debug( @@ -2047,15 +2051,17 @@ public class BucketCache implements BlockCache, HeapSize { + "Total blocks for file: {}. Checking for blocks pending cache in cache writer queue.", fileName.getName(), count.getValue(), dataBlockCount); if (ramCache.hasBlocksForFile(fileName.getName())) { + for (ReentrantReadWriteLock lock : locks) { +lock.readLock().unlock(); + } LOG.debug("There are still blocks pending caching for file {}. Will sleep 100ms " + "and try the verification again.", fileName.getName()); Thread.sleep(100); notifyFileCachingCompleted(fileName, totalBlockCount, dataBlockCount, size); } else { - LOG.info( -"We found only {} blocks cached from a total of {} for file {}, " - + "but no blocks pending caching. Maybe cache is full?", -count, dataBlockCount, fileName.getName()); + LOG.info("We found only {} blocks cached from a total of {} for file {}, " ++ "but no blocks pending caching. Maybe cache is full or evictions " ++ "happened concurrently to cache prefetch.", count, totalBlockCount, fileName); } } } catch (InterruptedException e) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestBlockEvictionOnRegionMovement.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestBlockEvictionOnRegionMovement.java index 8cd80e755cd..88b0b51131e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestBlockEvictionOnRegionMovement.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestBlockEvictionOnRegionMovement.java @@ -41,6 +41,7 @@ import
(hbase) branch branch-2.6 updated (c690b821ad1 -> a56126c2766)
This is an automated email from the ASF dual-hosted git repository. wchevreuil pushed a change to branch branch-2.6 in repository https://gitbox.apache.org/repos/asf/hbase.git from c690b821ad1 HBASE-28366 Mis-order of SCP and regionServerReport results into region inconsistencies (#5774) new ee4b44fa504 HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777) new a56126c2766 [ADDENDUM] HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777) (#5791) The 2 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: .../hadoop/hbase/io/hfile/bucket/BucketCache.java | 68 ++ .../hfile/TestBlockEvictionOnRegionMovement.java | 9 +-- .../io/hfile/bucket/TestBucketCachePersister.java | 25 .../io/hfile/bucket/TestPrefetchPersistence.java | 35 ++- 4 files changed, 68 insertions(+), 69 deletions(-)
(hbase) 02/02: [ADDENDUM] HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777) (#5791)
This is an automated email from the ASF dual-hosted git repository. wchevreuil pushed a commit to branch branch-2 in repository https://gitbox.apache.org/repos/asf/hbase.git commit 50eb7dafbcb5b6b8a5c9f83ac3bfe0b53bfc16f0 Author: Wellington Ramos Chevreuil AuthorDate: Fri Apr 5 10:56:06 2024 +0100 [ADDENDUM] HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777) (#5791) Signed-off-by: Peter Somogyi (cherry picked from commit d7566abd5de915e8f55a4f1f1939f6be38891657) --- .../hadoop/hbase/io/hfile/bucket/BucketCache.java | 48 ++ .../io/hfile/bucket/TestPrefetchPersistence.java | 35 +++- 2 files changed, 37 insertions(+), 46 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 19b158b9fc5..643f3d8d93d 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 @@ -736,7 +736,8 @@ public class BucketCache implements BlockCache, HeapSize { } else { return bucketEntryToUse.withWriteLock(offsetLock, () -> { if (backingMap.remove(cacheKey, bucketEntryToUse)) { - LOG.debug("removed key {} from back map in the evict process", cacheKey); + LOG.debug("removed key {} from back map with offset lock {} in the evict process", +cacheKey, bucketEntryToUse.offset()); blockEvicted(cacheKey, bucketEntryToUse, !existedInRamCache, evictedByEvictionProcess); return true; } @@ -1643,19 +1644,21 @@ public class BucketCache implements BlockCache, HeapSize { @Override public int evictBlocksByHfileName(String hfileName) { fileNotFullyCached(hfileName); -Set keySet = blocksByHFile.subSet(new BlockCacheKey(hfileName, Long.MIN_VALUE), - true, new BlockCacheKey(hfileName, Long.MAX_VALUE), true); - +Set keySet = getAllCacheKeysForFile(hfileName); int numEvicted = 0; for (BlockCacheKey key : keySet) { if (evictBlock(key)) { ++numEvicted; } } - return numEvicted; } + private Set getAllCacheKeysForFile(String hfileName) { +return blocksByHFile.subSet(new BlockCacheKey(hfileName, Long.MIN_VALUE), true, + new BlockCacheKey(hfileName, Long.MAX_VALUE), true); + } + /** * Used to group bucket entries into priority buckets. There will be a BucketEntryGroup for each * priority (single, multi, memory). Once bucketed, the eviction algorithm takes the appropriate @@ -2068,26 +2071,33 @@ public class BucketCache implements BlockCache, HeapSize { entry.getKey().getHfileName().equals(fileName.getName()) && entry.getKey().getBlockType().equals(BlockType.DATA) ) { - LOG.debug("found block for file {} in the backing map. Acquiring read lock for offset {}", -fileName.getName(), entry.getKey().getOffset()); - ReentrantReadWriteLock lock = offsetLock.getLock(entry.getKey().getOffset()); + long offsetToLock = entry.getValue().offset(); + LOG.debug("found block {} in the backing map. Acquiring read lock for offset {}", +entry.getKey(), offsetToLock); + ReentrantReadWriteLock lock = offsetLock.getLock(offsetToLock); lock.readLock().lock(); locks.add(lock); // rechecks the given key is still there (no eviction happened before the lock acquired) if (backingMap.containsKey(entry.getKey())) { count.increment(); + } else { +lock.readLock().unlock(); +locks.remove(lock); +LOG.debug("found block {}, but when locked and tried to count, it was gone."); } } }); + int metaCount = totalBlockCount - dataBlockCount; // BucketCache would only have data blocks if (dataBlockCount == count.getValue()) { LOG.debug("File {} has now been fully cached.", fileName); fileCacheCompleted(fileName, size); } else { LOG.debug( - "Prefetch executor completed for {}, but only {} blocks were cached. " -+ "Total blocks for file: {}. Checking for blocks pending cache in cache writer queue.", - fileName.getName(), count.getValue(), dataBlockCount); + "Prefetch executor completed for {}, but only {} data blocks were cached. " ++ "Total data blocks for file: {}. " ++ "Checking for blocks pending cache in cache writer queue.", + fileName, count.getValue(), dataBlockCount); if (ramCache.hasBlocksForFile(fileName.getName())) { for (ReentrantReadWriteLock lock : locks) { lock.readLock().unlock(); @@ -2096,11 +2106,17 @@ public class BucketCache
(hbase) 01/02: HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777)
This is an automated email from the ASF dual-hosted git repository. wchevreuil pushed a commit to branch branch-2 in repository https://gitbox.apache.org/repos/asf/hbase.git commit 113f04ea2cfe3583eec0ad31ee1bbdbf330888da Author: Wellington Ramos Chevreuil AuthorDate: Tue Apr 2 11:58:53 2024 +0100 HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777) Signed-off-by: Duo Zhang (cherry picked from commit c4ac2df041aa4795f91024b1e5dc8d4f5b6c048e) --- .../hadoop/hbase/io/hfile/bucket/BucketCache.java | 26 +- .../hfile/TestBlockEvictionOnRegionMovement.java | 9 .../io/hfile/bucket/TestBucketCachePersister.java | 25 +++-- 3 files changed, 34 insertions(+), 26 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 728946c1c18..19b158b9fc5 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 @@ -2058,26 +2058,30 @@ public class BucketCache implements BlockCache, HeapSize { // so we need to count all blocks for this file in the backing map under // a read lock for the block offset final List locks = new ArrayList<>(); -LOG.debug("Notifying caching completed for file {}, with total blocks {}", fileName, - dataBlockCount); +LOG.debug("Notifying caching completed for file {}, with total blocks {}, and data blocks {}", + fileName, totalBlockCount, dataBlockCount); try { final MutableInt count = new MutableInt(); LOG.debug("iterating over {} entries in the backing map", backingMap.size()); backingMap.entrySet().stream().forEach(entry -> { -if (entry.getKey().getHfileName().equals(fileName.getName())) { +if ( + entry.getKey().getHfileName().equals(fileName.getName()) +&& entry.getKey().getBlockType().equals(BlockType.DATA) +) { LOG.debug("found block for file {} in the backing map. Acquiring read lock for offset {}", fileName.getName(), entry.getKey().getOffset()); ReentrantReadWriteLock lock = offsetLock.getLock(entry.getKey().getOffset()); lock.readLock().lock(); locks.add(lock); + // rechecks the given key is still there (no eviction happened before the lock acquired) if (backingMap.containsKey(entry.getKey())) { count.increment(); } } }); - // We may either place only data blocks on the BucketCache or all type of blocks - if (dataBlockCount == count.getValue() || totalBlockCount == count.getValue()) { -LOG.debug("File {} has now been fully cached.", fileName.getName()); + // BucketCache would only have data blocks + if (dataBlockCount == count.getValue()) { +LOG.debug("File {} has now been fully cached.", fileName); fileCacheCompleted(fileName, size); } else { LOG.debug( @@ -2085,15 +2089,17 @@ public class BucketCache implements BlockCache, HeapSize { + "Total blocks for file: {}. Checking for blocks pending cache in cache writer queue.", fileName.getName(), count.getValue(), dataBlockCount); if (ramCache.hasBlocksForFile(fileName.getName())) { + for (ReentrantReadWriteLock lock : locks) { +lock.readLock().unlock(); + } LOG.debug("There are still blocks pending caching for file {}. Will sleep 100ms " + "and try the verification again.", fileName.getName()); Thread.sleep(100); notifyFileCachingCompleted(fileName, totalBlockCount, dataBlockCount, size); } else { - LOG.info( -"We found only {} blocks cached from a total of {} for file {}, " - + "but no blocks pending caching. Maybe cache is full?", -count, dataBlockCount, fileName.getName()); + LOG.info("We found only {} blocks cached from a total of {} for file {}, " ++ "but no blocks pending caching. Maybe cache is full or evictions " ++ "happened concurrently to cache prefetch.", count, totalBlockCount, fileName); } } } catch (InterruptedException e) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestBlockEvictionOnRegionMovement.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestBlockEvictionOnRegionMovement.java index 8cd80e755cd..88b0b51131e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestBlockEvictionOnRegionMovement.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestBlockEvictionOnRegionMovement.java @@ -41,6 +41,7 @@ import
(hbase) branch branch-2 updated (91519e8befd -> 50eb7dafbcb)
This is an automated email from the ASF dual-hosted git repository. wchevreuil pushed a change to branch branch-2 in repository https://gitbox.apache.org/repos/asf/hbase.git from 91519e8befd HBASE-28366 Mis-order of SCP and regionServerReport results into region inconsistencies (#5774) new 113f04ea2cf HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777) new 50eb7dafbcb [ADDENDUM] HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777) (#5791) The 2 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: .../hadoop/hbase/io/hfile/bucket/BucketCache.java | 68 ++ .../hfile/TestBlockEvictionOnRegionMovement.java | 9 +-- .../io/hfile/bucket/TestBucketCachePersister.java | 25 .../io/hfile/bucket/TestPrefetchPersistence.java | 35 ++- 4 files changed, 68 insertions(+), 69 deletions(-)
(hbase) branch branch-3 updated: [ADDENDUM] HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777) (#5791)
This is an automated email from the ASF dual-hosted git repository. wchevreuil pushed a commit to branch branch-3 in repository https://gitbox.apache.org/repos/asf/hbase.git The following commit(s) were added to refs/heads/branch-3 by this push: new d7566abd5de [ADDENDUM] HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777) (#5791) d7566abd5de is described below commit d7566abd5de915e8f55a4f1f1939f6be38891657 Author: Wellington Ramos Chevreuil AuthorDate: Fri Apr 5 10:56:06 2024 +0100 [ADDENDUM] HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777) (#5791) Signed-off-by: Peter Somogyi --- .../hadoop/hbase/io/hfile/bucket/BucketCache.java | 46 +++--- .../io/hfile/bucket/TestPrefetchPersistence.java | 35 +++- 2 files changed, 36 insertions(+), 45 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 9541939db94..71bfc757e51 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 @@ -747,7 +747,8 @@ public class BucketCache implements BlockCache, HeapSize { } else { return bucketEntryToUse.withWriteLock(offsetLock, () -> { if (backingMap.remove(cacheKey, bucketEntryToUse)) { - LOG.debug("removed key {} from back map in the evict process", cacheKey); + LOG.debug("removed key {} from back map with offset lock {} in the evict process", +cacheKey, bucketEntryToUse.offset()); blockEvicted(cacheKey, bucketEntryToUse, !existedInRamCache, evictedByEvictionProcess); return true; } @@ -1658,19 +1659,21 @@ public class BucketCache implements BlockCache, HeapSize { @Override public int evictBlocksByHfileName(String hfileName) { fileNotFullyCached(hfileName); -Set keySet = blocksByHFile.subSet(new BlockCacheKey(hfileName, Long.MIN_VALUE), - true, new BlockCacheKey(hfileName, Long.MAX_VALUE), true); - +Set keySet = getAllCacheKeysForFile(hfileName); int numEvicted = 0; for (BlockCacheKey key : keySet) { if (evictBlock(key)) { ++numEvicted; } } - return numEvicted; } + private Set getAllCacheKeysForFile(String hfileName) { +return blocksByHFile.subSet(new BlockCacheKey(hfileName, Long.MIN_VALUE), true, + new BlockCacheKey(hfileName, Long.MAX_VALUE), true); + } + /** * Used to group bucket entries into priority buckets. There will be a BucketEntryGroup for each * priority (single, multi, memory). Once bucketed, the eviction algorithm takes the appropriate @@ -2083,25 +2086,32 @@ public class BucketCache implements BlockCache, HeapSize { entry.getKey().getHfileName().equals(fileName.getName()) && entry.getKey().getBlockType().equals(BlockType.DATA) ) { - LOG.debug("found block for file {} in the backing map. Acquiring read lock for offset {}", -fileName, entry.getKey().getOffset()); - ReentrantReadWriteLock lock = offsetLock.getLock(entry.getKey().getOffset()); + long offsetToLock = entry.getValue().offset(); + LOG.debug("found block {} in the backing map. Acquiring read lock for offset {}", +entry.getKey(), offsetToLock); + ReentrantReadWriteLock lock = offsetLock.getLock(offsetToLock); lock.readLock().lock(); locks.add(lock); // rechecks the given key is still there (no eviction happened before the lock acquired) if (backingMap.containsKey(entry.getKey())) { count.increment(); + } else { +lock.readLock().unlock(); +locks.remove(lock); +LOG.debug("found block {}, but when locked and tried to count, it was gone."); } } }); + int metaCount = totalBlockCount - dataBlockCount; // BucketCache would only have data blocks if (dataBlockCount == count.getValue()) { LOG.debug("File {} has now been fully cached.", fileName); fileCacheCompleted(fileName, size); } else { LOG.debug( - "Prefetch executor completed for {}, but only {} blocks were cached. " -+ "Total blocks for file: {}. Checking for blocks pending cache in cache writer queue.", + "Prefetch executor completed for {}, but only {} data blocks were cached. " ++ "Total data blocks for file: {}. " ++ "Checking for blocks pending cache in cache writer queue.", fileName, count.getValue(), dataBlockCount); if (ramCache.hasBlocksForFile(fileName.getName())) { for (ReentrantReadWriteLock lock
(hbase) branch master updated: [ADDENDUM] HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777) (#5791)
This is an automated email from the ASF dual-hosted git repository. wchevreuil pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/hbase.git The following commit(s) were added to refs/heads/master by this push: new aea7e7c85cd [ADDENDUM] HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777) (#5791) aea7e7c85cd is described below commit aea7e7c85cdb8628fb03ead0f94d8e07ad49f067 Author: Wellington Ramos Chevreuil AuthorDate: Fri Apr 5 10:56:06 2024 +0100 [ADDENDUM] HBASE-28458 BucketCache.notifyFileCachingCompleted may incorrectly consider a file fully cached (#5777) (#5791) Signed-off-by: Peter Somogyi --- .../hadoop/hbase/io/hfile/bucket/BucketCache.java | 46 +++--- .../io/hfile/bucket/TestPrefetchPersistence.java | 35 +++- 2 files changed, 36 insertions(+), 45 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 9541939db94..71bfc757e51 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 @@ -747,7 +747,8 @@ public class BucketCache implements BlockCache, HeapSize { } else { return bucketEntryToUse.withWriteLock(offsetLock, () -> { if (backingMap.remove(cacheKey, bucketEntryToUse)) { - LOG.debug("removed key {} from back map in the evict process", cacheKey); + LOG.debug("removed key {} from back map with offset lock {} in the evict process", +cacheKey, bucketEntryToUse.offset()); blockEvicted(cacheKey, bucketEntryToUse, !existedInRamCache, evictedByEvictionProcess); return true; } @@ -1658,19 +1659,21 @@ public class BucketCache implements BlockCache, HeapSize { @Override public int evictBlocksByHfileName(String hfileName) { fileNotFullyCached(hfileName); -Set keySet = blocksByHFile.subSet(new BlockCacheKey(hfileName, Long.MIN_VALUE), - true, new BlockCacheKey(hfileName, Long.MAX_VALUE), true); - +Set keySet = getAllCacheKeysForFile(hfileName); int numEvicted = 0; for (BlockCacheKey key : keySet) { if (evictBlock(key)) { ++numEvicted; } } - return numEvicted; } + private Set getAllCacheKeysForFile(String hfileName) { +return blocksByHFile.subSet(new BlockCacheKey(hfileName, Long.MIN_VALUE), true, + new BlockCacheKey(hfileName, Long.MAX_VALUE), true); + } + /** * Used to group bucket entries into priority buckets. There will be a BucketEntryGroup for each * priority (single, multi, memory). Once bucketed, the eviction algorithm takes the appropriate @@ -2083,25 +2086,32 @@ public class BucketCache implements BlockCache, HeapSize { entry.getKey().getHfileName().equals(fileName.getName()) && entry.getKey().getBlockType().equals(BlockType.DATA) ) { - LOG.debug("found block for file {} in the backing map. Acquiring read lock for offset {}", -fileName, entry.getKey().getOffset()); - ReentrantReadWriteLock lock = offsetLock.getLock(entry.getKey().getOffset()); + long offsetToLock = entry.getValue().offset(); + LOG.debug("found block {} in the backing map. Acquiring read lock for offset {}", +entry.getKey(), offsetToLock); + ReentrantReadWriteLock lock = offsetLock.getLock(offsetToLock); lock.readLock().lock(); locks.add(lock); // rechecks the given key is still there (no eviction happened before the lock acquired) if (backingMap.containsKey(entry.getKey())) { count.increment(); + } else { +lock.readLock().unlock(); +locks.remove(lock); +LOG.debug("found block {}, but when locked and tried to count, it was gone."); } } }); + int metaCount = totalBlockCount - dataBlockCount; // BucketCache would only have data blocks if (dataBlockCount == count.getValue()) { LOG.debug("File {} has now been fully cached.", fileName); fileCacheCompleted(fileName, size); } else { LOG.debug( - "Prefetch executor completed for {}, but only {} blocks were cached. " -+ "Total blocks for file: {}. Checking for blocks pending cache in cache writer queue.", + "Prefetch executor completed for {}, but only {} data blocks were cached. " ++ "Total data blocks for file: {}. " ++ "Checking for blocks pending cache in cache writer queue.", fileName, count.getValue(), dataBlockCount); if (ramCache.hasBlocksForFile(fileName.getName())) { for (ReentrantReadWriteLock lock :