saurabhd336 commented on code in PR #11785:
URL: https://github.com/apache/pinot/pull/11785#discussion_r1359345141


##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java:
##########
@@ -700,19 +699,24 @@ public ServerReloadControllerJobStatusResponse 
getReloadJobStatus(
     
serverReloadControllerJobStatusResponse.setTotalServersQueried(serverUrls.size());
     
serverReloadControllerJobStatusResponse.setTotalServerCallsFailed(serviceResponse._failedResponseCount);
 
+    // Merge all server responses.
+    List<String> pendingSegments = new ArrayList<>();
+    int successCount = 0;
     for (Map.Entry<String, String> streamResponse : 
serviceResponse._httpResponses.entrySet()) {
       String responseString = streamResponse.getValue();
       try {
         ServerReloadControllerJobStatusResponse response =
             JsonUtils.stringToObject(responseString, 
ServerReloadControllerJobStatusResponse.class);
-        serverReloadControllerJobStatusResponse.setSuccessCount(
-            serverReloadControllerJobStatusResponse.getSuccessCount() + 
response.getSuccessCount());
+        successCount += response.getSuccessCount();
+        pendingSegments.addAll(response.getPendingSegments());

Review Comment:
   We can change this to `SegmentReloadStatusValue.class` while deserializing.
   
   But, I think since the agreed upon upgrade sequence is controller -> broker 
-> servers, if new fields are added to `SegmentReloadStatusValue`, in the 
controller we'd always need to ensure the newer fields are null checked before 
accessing. Since during upgrade server can be on older version. So moving to 
`SegmentReloadStatusValue` doesn't change the fact that we'll always need null 
checks for new fields added to the server's response class?
   



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