Jackie-Jiang commented on code in PR #14506:
URL: https://github.com/apache/pinot/pull/14506#discussion_r1857524350
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java:
##########
@@ -1557,6 +1558,7 @@ public void uploadToDeepStoreIfMissing(TableConfig
tableConfig, List<SegmentZKMe
// Randomly ask one server to upload
URI uri =
peerSegmentURIs.get(RANDOM.nextInt(peerSegmentURIs.size()));
+ String crcFromServer = getSegmentCrcFromServer(realtimeTableName,
segmentName, uri.toString());
Review Comment:
Currently we read crc and ask server to upload as 2 separate steps. A more
ideal solution would be to enhance the server upload API to return both segment
location (i.e. `tempSegmentDownloadUrl`) and also crc. The response can be a
json wrapping more info other than the download url.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java:
##########
@@ -1593,6 +1599,16 @@ public void uploadToDeepStoreIfMissing(TableConfig
tableConfig, List<SegmentZKMe
}
}
+ @VisibleForTesting
+ String getSegmentCrcFromServer(String tableNameWithType, String segmentName,
String endpoint) {
Review Comment:
Is this backward compatible when the server is still running the old pinot
version?
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
