otterc commented on code in PR #36601:
URL: https://github.com/apache/spark/pull/36601#discussion_r877340985
##########
core/src/test/scala/org/apache/spark/storage/ShuffleBlockFetcherIteratorSuite.scala:
##########
@@ -1786,4 +1786,32 @@ class ShuffleBlockFetcherIteratorSuite extends
SparkFunSuite with PrivateMethodT
ShuffleBlockId(0, 5, 2), ShuffleBlockId(0, 6, 2)))
}
+ test("SPARK-38987: failure to fetch corrupted shuffle block chunk should " +
Review Comment:
Nit: modify this name so it's clear that when corruption goes undetected
then it should throw fetch failure. Otherwise, it is confusing because one test
says we should fallback on corruption and the other says it should throw fetch
failure
##########
core/src/main/scala/org/apache/spark/storage/ShuffleBlockFetcherIterator.scala:
##########
@@ -1166,6 +1166,9 @@ final class ShuffleBlockFetcherIterator(
case ShuffleBlockBatchId(shuffleId, mapId, startReduceId, _) =>
throw SparkCoreErrors.fetchFailedError(address, shuffleId, mapId,
mapIndex, startReduceId,
msg, e)
+ case ShuffleBlockChunkId(shuffleId, _, reduceId, _) =>
+ SparkCoreErrors.fetchFailedError(address, shuffleId,
+ -1L, SHUFFLE_PUSH_MAP_ID, reduceId, msg, e)
Review Comment:
Nit: Can we use SHUFFLE_PUSH_MAP for mapId as well?
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
