jerqi commented on code in PR #276:
URL: https://github.com/apache/incubator-uniffle/pull/276#discussion_r1020858782
##########
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:
> > we won't retry this handler because we throw the exception here
>
> Why not? It just will not retry in current round, but it will retry in
next round.
If upper layer is LocalFileQuorumClientReadHanlder, we will retry another
handler when we throw an exception.
https://github.com/apache/incubator-uniffle/blob/4a3d2be36795df2eaf2edbe75ecc5816bf7eb87a/storage/src/main/java/org/apache/uniffle/storage/handler/impl/LocalFileQuorumClientReadHandler.java#L105
It seems that fail handler times won't reach the maxHandlerFailTimes in any
time. Is it necessary that we increase the failTimes? Could we change the
code?
```
if (++failTimes > maxHandlerFailTimes) {
isFinished = true;
}
throw e
```
to
```
isFinished = true;
throw e
```
Could you add a test case to explain when the fail time will reach the
maxHandlerFailTimes?
--
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]
