keith-turner commented on code in PR #5538:
URL: https://github.com/apache/accumulo/pull/5538#discussion_r2085371224


##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCacheImpl.java:
##########
@@ -690,25 +690,28 @@ private void lookupTablet(ClientContext context, Text 
row, LockCheckerSession lc
     metadataRow.append(row.getBytes(), 0, row.getLength());
     CachedTablet ptl = parent.findTablet(context, metadataRow, false, 
LocationNeed.REQUIRED);
 
-    if (ptl != null) {
-      // Only allow a single lookup at time per parent tablet. For example if 
a tables tablets are
-      // all stored in three metadata tablets, then that table could have up 
to three concurrent
-      // metadata lookups.
-      Timer timer = Timer.startNew();
-      try (var unused = lookupLocks.lock(ptl.getExtent())) {
-        // See if entry was added to cache by another thread while we were 
waiting on the lock
-        var cached = findTabletInCache(row);
-        if (cached != null && cached.getCreationTimer().startedAfter(timer)) {
-          // This cache entry was added after we started waiting on the lock 
so lets use it and not
-          // go to the metadata table. This means another thread was holding 
the lock and doing
-          // metadata lookups when we requested the lock.
-          return;
-        }
-        // Lookup tablets in metadata table and update cache. Also updating 
the cache while holding
-        // the lock is important as it ensures other threads that are waiting 
on the lock will see
-        // what this thread found and may be able to avoid metadata lookups.
-        lookupTablet(context, lcSession, ptl, metadataRow);
+    if (ptl == null) {
+      return;
+    }
+
+    CachedTablet now = findTabletInCache(row);
+    if (now != null) {
+      if (now.getTserverLocation().isPresent() && lcSession.checkLock(now) != 
null) {

Review Comment:
   In support of ondemand tablets, tablets w/o a location can be cached in 4.0 
(was not the case in 2.1).  So these checks that look for the presence of a 
location do not seem correct.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to