(accumulo) branch main updated: fixes race condition in ClientTabletCacheImpl (#5756)

[kturner]

This is an automated email from the ASF dual-hosted git repository.

kturner pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/main by this push:
     new 53d0297655 fixes race condition in ClientTabletCacheImpl (#5756)
53d0297655 is described below

commit 53d0297655b0d93f43344dd6ee698bc62b4616e9
Author: Keith Turner <ktur...@apache.org>
AuthorDate: Thu Jul 24 17:36:51 2025 -0400

    fixes race condition in ClientTabletCacheImpl (#5756)
    
    Fixes the following race condition.
    
     1. Thread 1 reads the cache and finds it stale/missing
     2. Thread 2 reads the cache and finds it stale/missing
     3. Thread 2 goes through the entire process of reading metadata and
        updating the cache
     4. Thread 1 does the pre lock read of the cache (this reread of the
        cache causes the race condition).
     5. Thread 1 does the post lock read of the cache and finds its the same
        and unessecairly reads the metadata table
    
    This change avoids reading from the cache twice before locking and
    updating and only reads once.  This way the cache entry used to make a
    decision after the lock is obtianed is the same one that was seen as
    stale.
    
    * format code
---
 .../core/clientImpl/ClientTabletCacheImpl.java        | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCacheImpl.java
 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCacheImpl.java
index 9d7345998d..8e4e89b435 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCacheImpl.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCacheImpl.java
@@ -682,9 +682,9 @@ public class ClientTabletCacheImpl extends 
ClientTabletCache {
     }
   }
 
-  private void lookupTablet(ClientContext context, Text row, 
LockCheckerSession lcSession)
-      throws AccumuloException, AccumuloSecurityException, 
TableNotFoundException,
-      InvalidTabletHostingRequestException {
+  private void lookupTablet(ClientContext context, Text row, 
LockCheckerSession lcSession,
+      CachedTablet before) throws AccumuloException, AccumuloSecurityException,
+      TableNotFoundException, InvalidTabletHostingRequestException {
     Text metadataRow = new Text(tableId.canonical());
     metadataRow.append(new byte[] {';'}, 0, 1);
     metadataRow.append(row.getBytes(), 0, row.getLength());
@@ -693,9 +693,10 @@ public class ClientTabletCacheImpl extends 
ClientTabletCache {
     if (ptl == null) {
       return;
     }
-    // detect if another thread populated cache while waiting for lock
-    CachedTablet before = findTabletInCache(row);
+
     try (var unused = lookupLocks.lock(ptl.getExtent())) {
+      // Now that the lock is acquired, detect if another thread populated 
cache since the last time
+      // the cache was read. If so then do not need to read from metadata 
store.
       CachedTablet after = findTabletInCache(row);
       if (after != null && after != before && lcSession.checkLock(after) != 
null) {
         return;
@@ -855,7 +856,7 @@ public class ClientTabletCacheImpl extends 
ClientTabletCache {
 
       // not in cache OR the cutoff timer was started after when the cached 
entry timer was started,
       // so obtain info from metadata table
-      tl = lookupTabletLocationAndCheckLock(context, row, lcSession);
+      tl = lookupTabletLocationAndCheckLock(context, row, lcSession, tl);
 
     }
 
@@ -863,9 +864,9 @@ public class ClientTabletCacheImpl extends 
ClientTabletCache {
   }
 
   private CachedTablet lookupTabletLocationAndCheckLock(ClientContext context, 
Text row,
-      LockCheckerSession lcSession) throws AccumuloException, 
AccumuloSecurityException,
-      TableNotFoundException, InvalidTabletHostingRequestException {
-    lookupTablet(context, row, lcSession);
+      LockCheckerSession lcSession, CachedTablet before) throws 
AccumuloException,
+      AccumuloSecurityException, TableNotFoundException, 
InvalidTabletHostingRequestException {
+    lookupTablet(context, row, lcSession, before);
     return lcSession.checkLock(findTabletInCache(row));
   }