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


##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCacheImpl.java:
##########
@@ -479,57 +445,44 @@ public List<Range> findTablets(ClientContext context, 
List<Range> ranges,
 
   @Override
   public void invalidateCache(KeyExtent failedExtent) {
-    wLock.lock();
-    try {
-      badExtents.add(failedExtent);
-    } finally {
-      wLock.unlock();
-    }
+    removeOverlapping(metaCache, failedExtent);
     if (log.isTraceEnabled()) {
       log.trace("Invalidated extent={}", failedExtent);
     }
   }
 
   @Override
   public void invalidateCache(Collection<KeyExtent> keySet) {
-    wLock.lock();
-    try {
-      badExtents.addAll(keySet);
-    } finally {
-      wLock.unlock();
-    }
+    keySet.forEach(extent -> removeOverlapping(metaCache, extent));
     if (log.isTraceEnabled()) {
       log.trace("Invalidated {} cache entries for table {}", keySet.size(), 
tableId);
     }
   }
 
   @Override
   public void invalidateCache(ClientContext context, String server) {
-
-    wLock.lock();
-    try {
-      badServers.add(server);
-    } finally {
-      wLock.unlock();
+    var iter = metaCache.values().iterator();
+    int removed = 0;
+    while (iter.hasNext()) {
+      var cachedTablet = iter.next();
+      if (cachedTablet.getTserverLocation().isPresent()
+          && cachedTablet.getTserverLocation().orElseThrow().equals(server)) {
+        iter.remove();
+        removed++;
+      }
     }

Review Comment:
   This code is related to removing servers from the cache.  While working on 
this have started to question the usefulness of removing entire servers from 
the cache.  Finally decided it may not be useful and removed it in #5431.
   
   That count that is being done used to be done in another place in the code 
that was deleted.  Decided to carry the count forward in the code that replaced 
the deleted code.   It depends on the circumstances, but sometimes it may be 
good to avoid using atomic integers for a counter in a loop.  I suspect doing 
this prevents optimizations like storing the counter in a cpu register (it has 
to be stored in memory probably because its volatile), but that only matters if 
the code is executed really frequently.  This code could traverse a lot of 
metadata entries, so the loop could traverse a lot of data.  None of this 
matters if we merge #5341 as this code would go away.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to