Vivek1106-04 commented on code in PR #57528:
URL: https://github.com/apache/spark/pull/57528#discussion_r3654177341
##########
core/src/main/scala/org/apache/spark/scheduler/MapStatus.scala:
##########
@@ -288,20 +288,21 @@ private[spark] object HighlyCompressedMapStatus {
.getOrElse(config.SHUFFLE_ACCURATE_BLOCK_THRESHOLD.defaultValue.get)
val threshold =
if (accurateBlockSkewedFactor > 0) {
- val sortedSizes = uncompressedSizes.sorted
- val medianSize: Long = Utils.median(sortedSizes, true)
val maxAccurateSkewedBlockNumber =
Math.min(
Option(SparkEnv.get)
.map(_.conf.get(config.SHUFFLE_MAX_ACCURATE_SKEWED_BLOCK_NUMBER))
.getOrElse(config.SHUFFLE_MAX_ACCURATE_SKEWED_BLOCK_NUMBER.defaultValue.get),
totalNumBlocks
)
+ // Only two order statistics are needed here, so they are selected in
O(totalNumBlocks)
+ // instead of sorting the sizes, which every map task would otherwise
pay for.
+ val sizes = uncompressedSizes.clone()
+ val medianSize: Long = Utils.medianInPlace(sizes)
+ val smallestAccurateSize =
+ Utils.nthSmallest(sizes, totalNumBlocks -
maxAccurateSkewedBlockNumber)
Review Comment:
You're right, and the reproduction is exact. Fixed in 2ced43b.
The cutoff size alone was never a cardinality bound, only a size bound. The
blocks strictly larger than the cutoff are now counted up front -- the
selection leaves them all in the top-K window, so that count is at most the
configured maximum -- and the remainder of the budget admits blocks of exactly
the cutoff size, in block index order, so the map status stays deterministic.
No strictly larger block is ever dropped in favour of a tied one, since only
ties are rationed. Blocks at or above `spark.shuffle.accurateBlockThreshold`
are still recorded unconditionally, so the mandatory path is unchanged.
The regression test is your example verbatim: 50000 partitions, 25001 blocks
of 1 KiB and 24999 of 8 KiB, stock defaults. Without the bound it fails with
`1024 did not equal 4600` -- all 24999 tied blocks get recorded and `avgSize`
collapses to the size of the small blocks, which is a neat second symptom of
the same bug.
--
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]
