Skip to content

Commit

Permalink
[SPARK-36389][CORE][SHUFFLE] Revert the change that accepts negative …
Browse files Browse the repository at this point in the history
…mapId in ShuffleBlockId

### What changes were proposed in this pull request?
With SPARK-32922, we added a change that ShuffleBlockId can have a negative mapId. This was to support push-based shuffle where -1 as mapId indicated a push-merged block. However with SPARK-32923, a different type of BlockId was introduced - ShuffleMergedId, but reverting the change to ShuffleBlockId was missed.

### Why are the changes needed?
This reverts the changes to `ShuffleBlockId` which will never have a negative mapId.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Modified the unit test to verify the newly added ShuffleMergedBlockId.

Closes apache#33616 from otterc/SPARK-36389.

Authored-by: Chandni Singh <singh.chandni@gmail.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
  • Loading branch information
otterc authored and dongjoon-hyun committed Aug 3, 2021
1 parent 8cb9cf3 commit 2712343
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
2 changes: 1 addition & 1 deletion core/src/main/scala/org/apache/spark/storage/BlockId.scala
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ class UnrecognizedBlockId(name: String)
@DeveloperApi
object BlockId {
val RDD = "rdd_([0-9]+)_([0-9]+)".r
val SHUFFLE = "shuffle_([0-9]+)_(-?[0-9]+)_([0-9]+)".r
val SHUFFLE = "shuffle_([0-9]+)_([0-9]+)_([0-9]+)".r
val SHUFFLE_BATCH = "shuffle_([0-9]+)_([0-9]+)_([0-9]+)_([0-9]+)".r
val SHUFFLE_DATA = "shuffle_([0-9]+)_([0-9]+)_([0-9]+).data".r
val SHUFFLE_INDEX = "shuffle_([0-9]+)_([0-9]+)_([0-9]+).index".r
Expand Down
10 changes: 5 additions & 5 deletions core/src/test/scala/org/apache/spark/storage/BlockIdSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -227,13 +227,13 @@ class BlockIdSuite extends SparkFunSuite {
}

test("merged shuffle id") {
val id = ShuffleBlockId(1, -1, 0)
assertSame(id, ShuffleBlockId(1, -1, 0))
assertDifferent(id, ShuffleBlockId(1, 1, 1))
assert(id.name === "shuffle_1_-1_0")
val id = ShuffleMergedBlockId(1, 2, 0)
assertSame(id, ShuffleMergedBlockId(1, 2, 0))
assertDifferent(id, ShuffleMergedBlockId(1, 3, 1))
assert(id.name === "shuffleMerged_1_2_0")
assert(id.asRDDId === None)
assert(id.shuffleId === 1)
assert(id.mapId === -1)
assert(id.shuffleMergeId === 2)
assert(id.reduceId === 0)
assertSame(id, BlockId(id.toString))
}
Expand Down

0 comments on commit 2712343

Please sign in to comment.