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


##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java:
##########
@@ -1088,9 +1091,32 @@ public NamespaceMapping getNamespaces() {
     return namespaces;
   }
 
-  public HashMap<TableId,ClientTabletCache> tabletCaches() {
+  public ClientTabletCache getTabletLocationCache(TableId tableId) {
     ensureOpen();
-    return tabletCaches;
+    synchronized (tabletLocationCache) {
+      return tabletLocationCache.computeIfAbsent(tableId, (TableId key) -> {
+        var mlo = new MetadataCachedTabletObtainer();
+        var lockChecker = getTServerLockChecker();
+        if (AccumuloTable.ROOT.tableId().equals(tableId)) {
+          return new RootClientTabletCache(lockChecker);
+        } else if (AccumuloTable.METADATA.tableId().equals(tableId)) {
+          return new ClientTabletCacheImpl(AccumuloTable.METADATA.tableId(),
+              getTabletLocationCache(AccumuloTable.ROOT.tableId()), mlo, 
lockChecker);

Review Comment:
   Inside computeIfAbset this function will call computeIfAbsent again w/ a 
different key which you are not supposed to do.  If the cache were per 
datalevel, then this would be safe (like if the top level cache was 
`Map<DataLevel, <Map<TableId,ClientTabletCache>>`



##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java:
##########
@@ -1088,9 +1091,32 @@ public NamespaceMapping getNamespaces() {
     return namespaces;
   }
 
-  public HashMap<TableId,ClientTabletCache> tabletCaches() {
+  public ClientTabletCache getTabletLocationCache(TableId tableId) {
     ensureOpen();
-    return tabletCaches;
+    synchronized (tabletLocationCache) {

Review Comment:
   Should be able to remove this sync block.  The javadoc for 
ConucrrentHashMap.computeIfAbsent says
   
   ```
   The entire method invocation is performed atomically, so the function is 
applied at most once per key.
   ```
   
   so for each tableId the lambda will run atomically.
   
   ```suggestion
   ```



-- 
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