jerqi commented on code in PR #276:
URL: https://github.com/apache/incubator-uniffle/pull/276#discussion_r1020883455


##########
storage/src/main/java/org/apache/uniffle/storage/handler/impl/DataSkippableReadHandler.java:
##########
@@ -67,8 +68,17 @@ public DataSkippableReadHandler(
 
   public ShuffleDataResult readShuffleData() {
     if (shuffleDataSegments.isEmpty()) {
-      ShuffleIndexResult shuffleIndexResult = readShuffleIndex();
+      ShuffleIndexResult shuffleIndexResult;
+      try {
+        shuffleIndexResult = readShuffleIndex();
+      } catch (Exception e) {
+        if (++failTimes > maxHandlerFailTimes) {
+          isFinished = true;
+        }
+        throw e;

Review Comment:
   I see your case. Terrific! But in my understanding the fail times of 
LocalFileQuorumClientReadHandler reach the  maxHandlerFailTimes, 
LocalFileQuorumClientReadHandler have three LocalFileClientReadHandlers. Every  
LocalFileClientReadHandler don't reach maxHandlerFailTimes. I mean that the 
fail times of LocalFileClientReadHandler  isn't meaningful. So could we change 
the code like my sayings? or the fail times of DataSkippableReadHandler is only 
used for HDFSFileClientReadHandler? 
   My concern is that the fail times of LocalFileClientReadHandler seems 
meaningless.



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