RexXiong commented on code in PR #3090:
URL: https://github.com/apache/celeborn/pull/3090#discussion_r2017297875
##########
client-spark/spark-3-4/src/main/scala/org/apache/spark/shuffle/celeborn/CelebornShuffleReader.scala:
##########
@@ -77,7 +77,27 @@ class CelebornShuffleReader[K, C](
val serializerInstance = newSerializerInstance(dep)
- val shuffleId = SparkUtils.celebornShuffleId(shuffleClient, handle,
context, false)
+ var shuffleId = handle.shuffleId
+ try {
+ shuffleId = SparkUtils.celebornShuffleId(shuffleClient, handle, context,
false)
Review Comment:
In this scenario, Celeborn would throw a CelebornException, so catching a
CelebornIOException is incorrect. Moreover, I believe we should not catch the
CelebornException itself, as we cannot identify the outcome when encountering
it. I propose adding a `bool success` field to the `PbGetShuffleIdResponse` to
indicate whether the shuffleId is available or not. This way, the client can
throw a FetchFailureException if necessary, thereby improving the handling and
expression of the situation.
--
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]
