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

Reply via email to