cloud-fan opened a new pull request, #59741: URL: https://github.com/apache/spark/pull/59741
### What changes were proposed in this pull request? This is a followup of https://github.com/apache/spark/pull/57742. Adaptive partial aggregation evaluates the compaction ratio at two points: periodically, every `minRows` processed rows, and right before the aggregation map would spill. The periodic check never decides on fewer than `minRows` rows, but the spill check decides on whatever the current map epoch has processed, which can be very few rows when the task is short on memory. This PR applies the same `minRows` floor to the spill check, on both the codegen path (`HashAggregateExec`) and the interpreted path (`TungstenAggregationIterator`). When the map fills before the current epoch has processed `minRows` rows, it spills as usual and the next epoch is judged on its own rows. `minRows = 0`, which disables the periodic check, still leaves the spill check without a floor. The `minRows` config doc and the SQL performance tuning guide are updated to match. ### Why are the changes needed? Pass-through cannot be undone within a task, so a decision made from a small sample is expensive when it is wrong. If duplicate keys are spread more widely than one small epoch, every epoch looks nearly distinct and the spill check bypasses, even though spilling and merging would have reduced the rows well. The task then sends roughly every input row to the shuffle instead of about one row per key. A missed bypass only costs the spill the operator would have done anyway, so waiting for `minRows` rows is the safer choice, and it makes the two check points consistent. ### Does this PR introduce _any_ user-facing change? No. Adaptive partial aggregation is new in the unreleased 4.4.0 and disabled by default. Within master, when the feature is enabled, a map that fills before its current epoch has processed `minRows` rows now spills instead of bypassing. ### How was this patch tested? Updated `AdaptivePartialAggregationSuite`: - The spill-check test is replaced by two tests. One checks that an epoch smaller than `minRows` spills and falls back to sort-based aggregation instead of bypassing. The other checks that, once past `minRows`, the spill check bypasses between two periodic checks, starting from the row that found the map full, on both the codegen and interpreted paths. - The exact-ratio boundary test now uses `minRows = 0`, so it still exercises the spill check without a floor. `build/sbt "sql/testOnly *AdaptivePartialAggregationSuite"` passes. The spill cases of `AdaptivePartialAggregationBenchmark` use `minRows = 0`, so their results are unaffected. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5.5) This pull request and its description were written by Isaac. -- 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]
