ivandika3 commented on code in PR #6292:
URL: https://github.com/apache/ozone/pull/6292#discussion_r1507013498


##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockInputStream.java:
##########
@@ -574,7 +574,20 @@ private boolean shouldRetryRead(IOException cause) throws 
IOException {
     } catch (Exception e) {
       throw new IOException(e);
     }
-    return retryAction.action == RetryPolicy.RetryAction.RetryDecision.RETRY;
+    if (retryAction.action == RetryPolicy.RetryAction.RetryDecision.RETRY) {

Review Comment:
   Just a thought: It might be nice if we can have a general RetryAction 
handler so that we don't need to duplicate the sleeping logic (similar codes is 
in `AbstractDataStreamOutput#shouldRetry`, `GrpcOmTranport#shouldRetry`, and 
`KeyOutputStream#handleRetry`)



##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockInputStream.java:
##########
@@ -574,7 +574,20 @@ private boolean shouldRetryRead(IOException cause) throws 
IOException {
     } catch (Exception e) {
       throw new IOException(e);
     }
-    return retryAction.action == RetryPolicy.RetryAction.RetryDecision.RETRY;
+    if (retryAction.action == RetryPolicy.RetryAction.RetryDecision.RETRY) {
+      if (retryAction.delayMillis > 0) {
+        try {
+          LOG.info("Retry read after {}ms", retryAction.delayMillis);

Review Comment:
   Nit: Will this cause a LOG flooding in case of persistent error across the 
blocks? For example, when one DN is down.



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