singhpk234 commented on code in PR #13619:
URL: https://github.com/apache/iceberg/pull/13619#discussion_r2279650769


##########
core/src/main/java/org/apache/iceberg/rest/ExponentialHttpRequestRetryStrategy.java:
##########
@@ -143,12 +141,20 @@ public boolean retryRequest(HttpResponse response, int 
execCount, HttpContext co
     HttpRequest request =
         context instanceof HttpCoreContext ? ((HttpCoreContext) 
context).getRequest() : null;
 
-    boolean shouldRetry =
-        execCount <= maxRetries
-            && (retriableCodes.contains(response.getCode())
-                || shouldRetryIdempotent(request, response.getCode()));
-
-    return shouldRetry;
+    boolean is503Retryable =
+        response.getCode() == HttpStatus.SC_SERVICE_UNAVAILABLE
+            && response.getFirstHeader(HttpHeaders.RETRY_AFTER) != null;
+
+    // A retry is permitted if all the following conditions are met:
+    // 1. The maximum retry count has not been exceeded.
+    // 2. The response code is considered retryable, for one of the following 
reasons:
+    //    - It's in a predefined list of retriable codes.
+    //    - The request is idempotent, and the response code indicates a retry 
is safe.
+    //    - The response code is '503 Service Unavailable' and includes a 
'Retry-After' header.
+    return execCount <= maxRetries
+        && (retriableCodes.contains(response.getCode())
+            || shouldRetryIdempotent(request, response.getCode())

Review Comment:
   IMHO its fine to retry even when the retry-after is not set for idempotent 
request, the 503 `retry-after` requirement for retry should come into the play 
for non idempotent request, thoughts ? 
   
   I agree, need to better rephrase the text for the spec above 
([here](https://github.com/apache/iceberg/pull/13619#discussion_r2279636821))



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to