klsince commented on code in PR #12936: URL: https://github.com/apache/pinot/pull/12936#discussion_r1567833872
########## pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java: ########## @@ -693,27 +693,22 @@ File downloadAndDecrypt(String segmentName, SegmentZKMetadata zkMetadata, File t } } - // not thread safe. Caller should invoke it with safe concurrency control. protected void downloadFromPeersWithoutStreaming(String segmentName, SegmentZKMetadata zkMetadata, File destTarFile) throws Exception { - Preconditions.checkState(_peerDownloadScheme != null, "Download peers require non null peer download scheme"); - List<URI> peerSegmentURIs = - PeerServerSegmentFinder.getPeerServerURIs(_helixManager, _tableNameWithType, segmentName, _peerDownloadScheme); - if (peerSegmentURIs.isEmpty()) { - String msg = String.format("segment %s doesn't have any peers", segmentName); - LOGGER.warn(msg); - // HelixStateTransitionHandler would catch the runtime exception and mark the segment state as Error - throw new RuntimeException(msg); - } + Preconditions.checkState(_peerDownloadScheme != null, "Peer download is not enabled for table: %s", + _tableNameWithType); try { - // Next download the segment from a randomly chosen server using configured scheme. - SegmentFetcherFactory.fetchAndDecryptSegmentToLocal(peerSegmentURIs, destTarFile, zkMetadata.getCrypterName()); - LOGGER.info("Fetched segment {} from peers: {} to: {} of size: {}", segmentName, peerSegmentURIs, destTarFile, + SegmentFetcherFactory.fetchAndDecryptSegmentToLocal(segmentName, _peerDownloadScheme, () -> { + List<URI> peerServerURIs = + PeerServerSegmentFinder.getPeerServerURIs(_helixManager, _tableNameWithType, segmentName, + _peerDownloadScheme); + Collections.shuffle(peerServerURIs); + return peerServerURIs; + }, destTarFile, zkMetadata.getCrypterName()); + _logger.info("Downloaded tarred segment: {} from peers to: {}, file length: {}", segmentName, destTarFile, Review Comment: nit: LOGGER or _logger? it it possible to get which peer the data was downloaded from and log it out here? -- 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