9aman commented on code in PR #15316:
URL: https://github.com/apache/pinot/pull/15316#discussion_r2007038286


##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java:
##########
@@ -564,77 +562,59 @@ private void doAddConsumingSegment(String segmentName)
   @Override
   public File downloadSegment(SegmentZKMetadata zkMetadata)
       throws Exception {
-    Preconditions.checkState(zkMetadata.getStatus() != Status.IN_PROGRESS,
-        "Segment: %s is still IN_PROGRESS and cannot be downloaded", 
zkMetadata.getSegmentName());
+    String segmentName = zkMetadata.getSegmentName();
+    Status status = zkMetadata.getStatus();
+    Preconditions.checkState(status != Status.IN_PROGRESS, "Segment: %s is 
still IN_PROGRESS and cannot be downloaded",
+        segmentName);
 
-    // Case: The commit protocol has completed, and the segment is ready to be 
downloaded either
-    // from deep storage or from a peer (if peer-to-peer download is enabled).
-    if (zkMetadata.getStatus() == Status.DONE) {
+    // The commit protocol has completed, and the segment is ready to be 
downloaded either from deep storage or from a
+    // peer (if peer-to-peer download is enabled).
+    if (status.isCompleted()) {
       return super.downloadSegment(zkMetadata);
     }
 
     // The segment status is COMMITTING, indicating that the segment commit 
process is incomplete.
     // Attempting a waited download within the configured time limit.
-    long downloadTimeoutMilliseconds =
-        
getDownloadTimeOutMilliseconds(ZKMetadataProvider.getTableConfig(_propertyStore,
 _tableNameWithType));
-    final long startTime = System.currentTimeMillis();
-    List<URI> onlineServerURIs;
-    while (System.currentTimeMillis() - startTime < 
downloadTimeoutMilliseconds) {
+    Preconditions.checkState(status == Status.COMMITTING, "Invalid status: %s 
for segment: %s", status, segmentName);
+    TableConfig tableConfig = 
ZKMetadataProvider.getTableConfig(_propertyStore, _tableNameWithType);
+    Preconditions.checkState(tableConfig != null, "Failed to find table config 
for table: %s", _tableNameWithType);
+    long downloadTimeoutMs = getDownloadTimeoutMs(tableConfig);
+    long deadlineMs = System.currentTimeMillis() + downloadTimeoutMs;
+    while (System.currentTimeMillis() < deadlineMs) {
       // ZK Metadata may change during segment download process; fetch it on 
every retry.
-      zkMetadata = fetchZKMetadata(zkMetadata.getSegmentName());
-
-      if (zkMetadata.getDownloadUrl() != null) {
-        // The downloadSegment() will throw an exception in case there are 
some genuine issues.
-        // We don't want to retry in those scenarios and will throw an 
exception
-        return downloadSegmentFromDeepStore(zkMetadata);
-      }
-
-      if (_peerDownloadScheme != null) {

Review Comment:
   Addressed here: https://github.com/apache/pinot/pull/15316/files#r2007034972



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