ankitsultana commented on code in PR #12317:
URL: https://github.com/apache/pinot/pull/12317#discussion_r1482212043


##########
pinot-common/src/main/java/org/apache/pinot/common/utils/fetcher/BaseSegmentFetcher.java:
##########
@@ -109,6 +110,40 @@ public File fetchUntarSegmentToLocalStreamed(URI uri, File 
dest, long rateLimit,
     throw new UnsupportedOperationException();
   }
 
+  /**
+   * @param segmentName the name of the segment to fetch.
+   * @param uriSupplier the supplier to the list of segment download uris.
+   * @param dest        The destination to put the downloaded segment.
+   * throws exception when the segment fetch fails after all attempts are 
exhausted or other runtime exceptions occur.

Review Comment:
   nit: `throws` ==> `@throws`



##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java:
##########
@@ -658,11 +659,15 @@ private void downloadSegmentFromPeer(String segmentName, 
String downloadScheme,
     try {
       tempRootDir = getTmpSegmentDataDir("tmp-" + segmentName + "." + 
System.currentTimeMillis());
       File segmentTarFile = new File(tempRootDir, segmentName + 
TarGzCompressionUtils.TAR_GZ_FILE_EXTENSION);
-      // First find servers hosting the segment in a ONLINE state.
-      List<URI> peerSegmentURIs = 
PeerServerSegmentFinder.getPeerServerURIs(segmentName, downloadScheme, 
_helixManager);
-      // Next download the segment from a randomly chosen server using 
configured scheme.
-      
SegmentFetcherFactory.getSegmentFetcher(downloadScheme).fetchSegmentToLocal(peerSegmentURIs,
 segmentTarFile);
-      _logger.info("Fetched segment {} from: {} to: {} of size: {}", 
segmentName, peerSegmentURIs, segmentTarFile,
+      // Next download the segment from a randomly chosen server using 
configured download scheme (http or https).
+      
SegmentFetcherFactory.getSegmentFetcher(downloadScheme).fetchSegmentToLocal(segmentName,
+          () -> {
+            List<URI> peerServerURIs =
+                PeerServerSegmentFinder.getPeerServerURIs(segmentName, 
downloadScheme, _helixManager);
+            Collections.shuffle(peerServerURIs);
+            return peerServerURIs;
+          }, segmentTarFile);
+      _logger.info("Fetched segment {} from: {} to: {} of size: {}", 
segmentName, segmentTarFile,

Review Comment:
   nit: number of args and `{}` placeholders don't match



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

Reply via email to