otterc commented on a change in pull request #35325:
URL: https://github.com/apache/spark/pull/35325#discussion_r793258742
##########
File path:
common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/RemoteBlockPushResolverSuite.java
##########
@@ -1161,8 +1161,8 @@ public void testFinalizeOfDeterminateShuffle() throws
IOException {
RemoteBlockPushResolver.AppShuffleInfo appShuffleInfo =
pushResolver.validateAndGetAppShuffleInfo(TEST_APP);
- assertTrue("Metadata of determinate shuffle should be removed after
finalize shuffle"
- + " merge", appShuffleInfo.getShuffles().get(0) == null);
+ assertTrue("Determinate shuffle should be marked finalized"
+ + " merge", appShuffleInfo.getShuffles().get(0).isFinalized());
Review comment:
Nit: remove ` merge`
##########
File path:
common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java
##########
@@ -1008,20 +996,28 @@ AppShufflePartitionInfo getPartitionInfo() {
* required for the shuffles of indeterminate stages.
*/
public static class AppShuffleMergePartitionsInfo {
+ // ConcurrentHashMap doesn't allow null for keys or values which is why
this is required.
+ // Marker to identify finalized indeterminate shuffle partitions in the
case of indeterminate
Review comment:
Nit: remove indeterminate
##########
File path:
common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/RemoteBlockPushResolverSuite.java
##########
@@ -1287,6 +1287,79 @@ void closeAndDeletePartitionFiles(Map<Integer,
AppShufflePartitionInfo> partitio
+ " up", appShuffleInfo.getMergedShuffleDataFile(0, 4, 0).exists());
}
+ @Test
+ public void
testFinalizationResultIsEmptyWhenTheServerDidNotReceiveAnyBlocks() {
+ //shuffle 1 0 is finalized even though the server didn't receive any
blocks for it.
+ MergeStatuses statuses = pushResolver.finalizeShuffleMerge(
+ new FinalizeShuffleMerge(TEST_APP, NO_ATTEMPT_ID, 1, 0));
+ assertEquals("no partitions were merged", 0, statuses.reduceIds.length);
+ RemoteBlockPushResolver.AppShuffleInfo appShuffleInfo =
+ pushResolver.validateAndGetAppShuffleInfo(TEST_APP);
+ assertTrue("Determinate shuffle should be marked finalized merge",
Review comment:
Nit: fix the msg
--
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]
