summaryzb commented on code in PR #2442:
URL: https://github.com/apache/uniffle/pull/2442#discussion_r2041971692


##########
internal-client/src/main/java/org/apache/uniffle/client/factory/ShuffleServerClientFactory.java:
##########
@@ -59,20 +59,18 @@ private ShuffleServerClient createShuffleServerClient(
     }
   }
 
-  public synchronized ShuffleServerClient getShuffleServerClient(
+  public ShuffleServerClient getShuffleServerClient(
       String clientType, ShuffleServerInfo shuffleServerInfo) {
     return getShuffleServerClient(clientType, shuffleServerInfo, new 
RssConf());
   }
 
-  public synchronized ShuffleServerClient getShuffleServerClient(
+  public ShuffleServerClient getShuffleServerClient(
       String clientType, ShuffleServerInfo shuffleServerInfo, RssConf rssConf) 
{
-    clients.putIfAbsent(clientType, JavaUtils.newConcurrentMap());
-    Map<ShuffleServerInfo, ShuffleServerClient> serverToClients = 
clients.get(clientType);
-    if (serverToClients.get(shuffleServerInfo) == null) {
-      serverToClients.put(
-          shuffleServerInfo, createShuffleServerClient(clientType, 
shuffleServerInfo, rssConf));
-    }
-    return serverToClients.get(shuffleServerInfo);
+    Map<ShuffleServerInfo, ShuffleServerClient> serverToClients =
+        clients.computeIfAbsent(clientType, key -> 
JavaUtils.newConcurrentMap());
+    return serverToClients.computeIfAbsent(

Review Comment:
   How about use cache to generate the client, without caring about whether the 
corresponding server is useless or available
   ```
   cache = Caffeine.newBuilder()
       .expireAfterAccess(expireTimeInMs, TimeUnit.MILLISECONDS)
       .scheduler(xxx)
       .removalListener((RemovalListener<> xxx)
       .build(new CacheLoader<<ClientType, ShuffleServerInfo, RssConf>, 
ShuffleServerClient>>() {
                       @Override
                       public @Nullable ShuffleServerClient load(xxx) {
                           return createShuffleServerClient(clientType, 
shuffleServerInfo, rssConf))
                       }
                   });
   
   cache.get(clientType, shuffleServerInfo, rssConf)
   ```



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to