cloud-fan commented on code in PR #59741:
URL: https://github.com/apache/spark/pull/59741#discussion_r4359387280
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/aggregate/AdaptivePartialAggregationSuite.scala:
##########
@@ -1139,26 +1139,61 @@ class AdaptivePartialAggregationSuite extends QueryTest
with SharedSparkSession
}
}
- test("the spill check fires when the map would spill on high-cardinality
input") {
- // Force the regular map to fall back quickly. High-cardinality input that
reaches the fallback
- // point should bypass via the spill check rather than spilling. Use a
`minRows` larger than the
- // input so the periodic check cannot fire first and the spill check is
the one exercised.
+ test("the spill check does not bypass an epoch smaller than minRows") {
+ // The spill check, like the periodic check, judges an epoch only once it
holds `minRows` rows,
+ // because the bypass cannot be undone within the task. Force the regular
map to fall back
+ // every 16 rows with a `minRows` larger than the input: every epoch is
below the floor, so
Review Comment:
Thanks, done in 3714a80a90f: `minRows` is now 32, and the comment says each
epoch, rather than the whole input, stays below the floor. To confirm the test
now pins the per-epoch scope, I temporarily made the spill-check floor count
rows across the whole task on both paths, and the test fails (`wholeStage=true
twoLevelMap=true 168 did not equal 0`).
One correction to the arithmetic: the forced-fallback counter increments for
every row, including the rows the fast map absorbs, so each epoch is exactly 16
rows in all four modes rather than up to 20. That is why the task-wide floor
bypasses from row 32 (200 - 32 = 168 rows).
The same commit also fixes the stale "(huge minRows)" benchmark comment from
the review summary.
--
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]
