aishikbh commented on code in PR #13232: URL: https://github.com/apache/pinot/pull/13232#discussion_r1618222815
########## pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/retention/RetentionManager.java: ########## @@ -55,7 +55,7 @@ */ public class RetentionManager extends ControllerPeriodicTask<Void> { public static final long OLD_LLC_SEGMENTS_RETENTION_IN_MILLIS = TimeUnit.DAYS.toMillis(5L); - private static final RetryPolicy DEFAULT_RETRY_POLICY = RetryPolicies.exponentialBackoffRetryPolicy(5, 1000L, 2.0f); + private static final RetryPolicy DEFAULT_RETRY_POLICY = RetryPolicies.randomDelayRetryPolicy(20, 100L, 200L); Review Comment: Okay on further inspection, I do not see much value in adding random delays with the current parameters since the maximum wait time will be `200*20 = 4000ms` where the minimum delay now with exponential backoff is `2000 + 4000 + 8000 + 16000 + 32000 = 62000ms` and the update is failing even then. I think increasing the number of attempts in exponential delay should be sufficient to make it a bit more resilient. What do you think @Jackie-Jiang @snleee ? -- 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