Repository navigation
Re-enable the two ignored CometAggregateSuite metric tests after the DataFusion 55 peak_mem_used change #5703
Description
Describe the bug
Two tests in CometAggregateSuite are disabled on the DataFusion 55 branch with only a prose TODO and no tracking issue:
// TODO: To be addressed after DF 55 migration
ignore("grouped aggregate metrics are forwarded without fabricating global metrics") {
...
// TODO: To be addressed after DF 55 migration
ignore("range sampling does not report grouped aggregate metrics") {They assert on spill_count, spilled_bytes, spilled_rows and peak_mem_used. peak_mem_used accounting changed in apache/datafusion#22710, which is what broke them.
Steps to reproduce
Un-ignore either test on the df55 branch.
Expected behavior
Either the assertions are adapted to DataFusion 55's accounting, or the metric is reported the way the tests expect. Either way the skips should point at this issue rather than a prose TODO, which is easy to lose once the branch merges.
Additional context
@sunchao offered to take this as a follow-up after #5262 merges. Related: the CometTPCHQuerySuite off-heap bump tracked separately.
Activity
andygrove commentedon Sep 5, 2026
Tracked under #5709 ([EPIC] DataFusion 55 upgrade follow-ups).
ErikBPF commentedon Sep 13, 2026
I reproduced the two ignored aggregate-metric tests on
f69c4c81b9429e327ea95658530ae4ed4ed19635, with DataFusion 55.1.0, Spark4.1.3, and a release native library. I re-enabled only those tests and
explicitly enabled
CometConf.COMET_RESPECT_DATAFUSION_CONFIGSin eachtest's configuration.
The sole difference between the two diagnostic runs was
spark.comet.datafusion.execution.enable_migration_aggregate:true(default)peak_mem_usedassertionsfalseThe grouped test reported
Expected peak aggregate memory for modes List(Final). The sampling test passed the sampling-exclusion checks andfailed at the positive peak assertion after execution.
Source inspection matches this result: the migrated implementation in
DataFusion's
aggregates/hash_stream.rsuses memory reservations and spillmetrics but does not register the
peak_mem_usedgauge. The legacy groupedstream registers and updates it; Comet forwards registered metrics.
This points to missing peak-memory instrumentation in migrated aggregation.
It does not show missing memory accounting or prove increased memory use.
Disabling migration was only a diagnostic control. I have left the tests
and production defaults unchanged, since restoration of these metrics is
already being coordinated.
Correct the metrics bullet in the Comet 1.1.0 post
docs: note that the peak native aggregate memory metric is not report…
Add Apache DataFusion Comet 1.1.0 release blog post (#205)