duanmeng commented on code in PR #276:
URL: https://github.com/apache/incubator-uniffle/pull/276#discussion_r1012719982
##########
storage/src/main/java/org/apache/uniffle/storage/handler/impl/DataSkippableReadHandler.java:
##########
@@ -59,8 +60,17 @@ public DataSkippableReadHandler(
public ShuffleDataResult readShuffleData() {
if (shuffleDataSegments.isEmpty()) {
- ShuffleIndexResult shuffleIndexResult = readShuffleIndex();
+ ShuffleIndexResult shuffleIndexResult;
+ try {
+ shuffleIndexResult = readShuffleIndex();
+ } catch (Exception e) {
+ if (++failTimes > maxHanderFailTimes) {
+ isFinished = true;
+ }
+ throw e;
Review Comment:
So it may loop the handlers a second time, skip the finished ones and some
handlers may be still unfinished for the maxHandlerFailTimes might be greater
than 2?
I have two suggestions, could you help to review?
- Do the retry in the upper layer and only keep state data such as the
finish flag and `segmentIndex` in the low-level handlers, and derive the finish
flag from the `segmentIndex`, which can be also used as a checkpoint for retry.
- **Do Not** expose the retry logic to the `ShuffleClient`, make it cohesive
in the `ClientReadHandler` and can be configured.
--
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]
