kuszz commented on code in PR #2138:
URL: 
https://github.com/apache/incubator-uniffle/pull/2138#discussion_r1778102397


##########
internal-client/src/main/java/org/apache/uniffle/client/impl/grpc/CoordinatorGrpcRetryableClient.java:
##########
@@ -64,7 +65,10 @@ public CoordinatorGrpcRetryableClient(
         ThreadUtils.getDaemonFixedThreadPool(heartBeatThreadNum, 
"client-heartbeat");
   }
 
-  public void sendAppHeartBeat(RssAppHeartBeatRequest request, long timeoutMs) 
{
+  @Override
+  public RssAppHeartBeatResponse sendAppHeartBeat(RssAppHeartBeatRequest 
request) {

Review Comment:
   This method seems only uses a thread pool to send heartbeats in parallel, 
may be we don't need to modify the name? 



##########
server/src/main/java/org/apache/uniffle/server/RegisterHeartBeat.java:
##########
@@ -58,11 +54,12 @@ public RegisterHeartBeat(ShuffleServer shuffleServer) {
     CoordinatorClientFactory factory = CoordinatorClientFactory.getInstance();
     this.coordinatorClients =
         factory.createCoordinatorClient(
-            conf.get(ShuffleServerConf.RSS_COORDINATOR_CLIENT_TYPE), 
this.coordinatorQuorum);
+            conf.get(ShuffleServerConf.RSS_COORDINATOR_CLIENT_TYPE),
+            this.coordinatorQuorum,
+            0,

Review Comment:
   public synchronized CoordinatorGrpcRetryableClient createCoordinatorClient(
         ClientType clientType,
         String coordinators,
         int heartBeatThreadNum)    
   would conflict with
   public synchronized CoordinatorClient createCoordinatorClient(
         ClientType clientType, String host, int port) ,
   so it's not convenient to add



##########
server/src/main/java/org/apache/uniffle/server/RegisterHeartBeat.java:
##########
@@ -45,10 +42,9 @@ public class RegisterHeartBeat {
   private final long heartBeatInterval;
   private final ShuffleServer shuffleServer;
   private final String coordinatorQuorum;
-  private final List<CoordinatorClient> coordinatorClients;
+  private final CoordinatorGrpcRetryableClient coordinatorClients;

Review Comment:
   done



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