swaminathanmanish commented on code in PR #15638:
URL: https://github.com/apache/pinot/pull/15638#discussion_r2060482197


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/SegmentDeletionManager.java:
##########
@@ -459,6 +466,32 @@ public void 
removeAgedDeletedSegments(LeadControllerManager leadControllerManage
     }
   }
 
+  private static boolean deleteWithTimeout(PinotFS pinotFS, URI targetURI, 
boolean forceDelete, long timeout,

Review Comment:
   Does it make sense to add retries or we would anyway come back to file again 
in the next run. 



##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/SegmentDeletionManager.java:
##########
@@ -459,6 +466,32 @@ public void 
removeAgedDeletedSegments(LeadControllerManager leadControllerManage
     }
   }
 
+  private static boolean deleteWithTimeout(PinotFS pinotFS, URI targetURI, 
boolean forceDelete, long timeout,
+      TimeUnit timeUnit) {
+    CompletableFuture<Boolean> deleteFuture = CompletableFuture.supplyAsync(() 
-> {

Review Comment:
   Eventually this should be in pinotFS itself. I guess we need 
delegator/composition pattern, where we have resilientPinotFs or something that 
delegates calls to underlying pinotFs but wraps around the calls with 
timeout/retries. Alternatively every implementation wraps around its calls from 
a shared timeout & retry wrapper utility. 
   



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