(hbase-site) branch asf-site updated: INFRA-10751 Empty commit

2024-04-05 Thread git-site-role
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)

2024-04-05 Thread wchevreuil
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)

2024-04-05 Thread wchevreuil
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)

2024-04-05 Thread wchevreuil
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)

2024-04-05 Thread wchevreuil
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)

2024-04-05 Thread wchevreuil
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)

2024-04-05 Thread wchevreuil
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)

2024-04-05 Thread wchevreuil
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)

2024-04-05 Thread wchevreuil
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 :