otterc commented on a change in pull request #33034:
URL: https://github.com/apache/spark/pull/33034#discussion_r676144745
##########
File path:
common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExternalBlockHandler.java
##########
@@ -380,9 +381,10 @@ protected Ratio getRatio() {
} else if (blockId0Parts.length == 4 &&
blockId0Parts[0].equals(SHUFFLE_CHUNK_ID)) {
requestForMergedBlockChunks = true;
final int shuffleId = Integer.parseInt(blockId0Parts[1]);
+ final int shuffleMergeId = Integer.parseInt(blockId0Parts[2]);
final int[] reduceIdAndChunkIds = shuffleMapIdAndReduceIds(blockIds,
shuffleId);
Review comment:
Tests are not run because I think they even fail to compile. This is
missing modifications to `shuffleMapIdAndReduceIds(...)` to handle introduction
of this new field. This is the code in `shuffleMapIdAndReduceIds` which will
throw the exception.
```
if (blockIdParts.length != 4
|| (!requestForMergedBlockChunks &&
!blockIdParts[0].equals(SHUFFLE_BLOCK_ID))
|| (requestForMergedBlockChunks &&
!blockIdParts[0].equals(SHUFFLE_CHUNK_ID))) {
throw new IllegalArgumentException("Unexpected shuffle block id
format: " + blockIds[i]);
}
```
I just modified one of the tests locally and added another field to
shuffleChunk block Id and it failed with
```
java.lang.IllegalArgumentException: Unexpected shuffle block id format:
shuffleChunk_0_1_0_1
at
org.apache.spark.network.shuffle.ExternalBlockHandler$ManagedBufferIterator.shuffleMapIdAndReduceIds(ExternalBlockHandler.java:414)
```
--
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]
