You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "sunchao (via GitHub)" <gi...@apache.org> on 2026/08/23 04:07:29 UTC

[PR] Expose native aggregate spill and memory metrics [datafusion-comet]

sunchao opened a new pull request, #5423:
URL: https://github.com/apache/datafusion-comet/pull/5423

   ## Why are the changes needed?
   
   Native grouped aggregate spill and memory metrics are not exposed consistently in Spark SQL execution metrics, making memory-pressure diagnosis difficult.
   
   ## What changes were proposed in this PR?
   
   Expose native grouped aggregate spill count, spilled bytes, spilled rows, and peak memory usage. Attribute metrics to the correct aggregate operator and exclude sampling-only aggregates.
   
   ## How was this PR tested?
   
   Ran focused aggregate spill regression tests and Spotless formatting checks.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] feat: expose native aggregate spill and memory metrics [datafusion-comet]

Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #5423:
URL: https://github.com/apache/datafusion-comet/pull/5423#discussion_r3846119150


##########
spark/src/main/scala/org/apache/spark/sql/comet/CometMetricNode.scala:
##########
@@ -61,6 +61,17 @@ case class CometMetricNode(metrics: Map[String, SQLMetric], children: Seq[CometM
     else children.flatMap(_.leafNodes)
   }
 
+  private[comet] def withoutAggregateMetrics(plan: SparkPlan): CometMetricNode =

Review Comment:
   it would be useful to comment the reason aggregation metrics are excluded



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] feat: expose native aggregate spill and memory metrics [datafusion-comet]

Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao merged PR #5423:
URL: https://github.com/apache/datafusion-comet/pull/5423


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] feat: expose native aggregate spill and memory metrics [datafusion-comet]

Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on code in PR #5423:
URL: https://github.com/apache/datafusion-comet/pull/5423#discussion_r3846536589


##########
spark/src/main/scala/org/apache/spark/sql/comet/CometMetricNode.scala:
##########
@@ -61,6 +61,17 @@ case class CometMetricNode(metrics: Map[String, SQLMetric], children: Seq[CometM
     else children.flatMap(_.leafNodes)
   }
 
+  private[comet] def withoutAggregateMetrics(plan: SparkPlan): CometMetricNode =

Review Comment:
   Good point. Range partitioning executes the native child once to sample partition boundaries and then again for the real shuffle. If aggregate metrics are updated in both passes, spill count, spilled bytes/rows, and peak native memory are counted twice. The sampling pass therefore suppresses only aggregate metrics; scan/input and other operator metrics remain intact, and the real shuffle execution reports aggregate metrics normally. This is why the filtering helper needs an explanatory comment.



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] feat: expose native aggregate spill and memory metrics [datafusion-comet]

Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on PR #5423:
URL: https://github.com/apache/datafusion-comet/pull/5423#issuecomment-5413982010

   Merged, thanks @comphead for the review!


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org