Jackie-Jiang commented on code in PR #12886:
URL: https://github.com/apache/pinot/pull/12886#discussion_r1577051333


##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java:
##########
@@ -608,172 +721,153 @@ protected SegmentDataManager unregisterSegment(String 
segmentName) {
     }
   }
 
-  protected boolean allowDownload(String segmentName, SegmentZKMetadata 
zkMetadata) {
-    return true;
-  }
-
-  protected File downloadSegment(String segmentName, SegmentZKMetadata 
zkMetadata)
-      throws Exception {
-    // TODO: may support download from peer servers for RealTime table.
-    return downloadSegmentFromDeepStore(segmentName, zkMetadata);
-  }
-
-  private File downloadSegmentFromDeepStore(String segmentName, 
SegmentZKMetadata zkMetadata)
+  /**
+   * Downloads an immutable segment into the index directory.
+   * Segment can be downloaded from deep store or from peer servers. 
Downloaded segment might be compressed or
+   * encrypted, and this method takes care of decompressing and decrypting the 
segment.
+   */
+  protected File downloadSegment(SegmentZKMetadata zkMetadata)
       throws Exception {
-    File tempRootDir = getTmpSegmentDataDir("tmp-" + segmentName + "-" + 
UUID.randomUUID());
-    if (_isStreamSegmentDownloadUntar && zkMetadata.getCrypterName() == null) {
-      try {
-        File untaredSegDir = downloadAndStreamUntarWithRateLimit(segmentName, 
zkMetadata, tempRootDir,
-            _streamSegmentDownloadUntarRateLimitBytesPerSec);
-        return moveSegment(segmentName, untaredSegDir);
-      } finally {
-        FileUtils.deleteQuietly(tempRootDir);
-      }
-    } else {
-      try {
-        File tarFile = downloadAndDecrypt(segmentName, zkMetadata, 
tempRootDir);
-        return untarAndMoveSegment(segmentName, tarFile, tempRootDir);
-      } finally {
-        FileUtils.deleteQuietly(tempRootDir);
-      }
-    }
-  }
-
-  private File moveSegment(String segmentName, File untaredSegDir)
-      throws IOException {
+    String segmentName = zkMetadata.getSegmentName();
+    String downloadUrl = zkMetadata.getDownloadUrl();
+    Preconditions.checkState(downloadUrl != null,
+        "Failed to find download URL in ZK metadata for segment: %s of table: 
%s", segmentName, _tableNameWithType);
     try {
-      File indexDir = getSegmentDataDir(segmentName);
-      FileUtils.deleteDirectory(indexDir);
-      FileUtils.moveDirectory(untaredSegDir, indexDir);
-      return indexDir;
+      if 
(!CommonConstants.Segment.METADATA_URI_FOR_PEER_DOWNLOAD.equals(downloadUrl)) {
+        try {
+          return downloadSegmentFromDeepStore(zkMetadata);
+        } catch (Exception e) {
+          if (_peerDownloadScheme != null) {

Review Comment:
   Don't follow the second part



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