[GitHub] [spark] cloud-fan commented on a change in pull request #32242: [SPARK-35141][SQL] Support two level of hash maps for final hash aggregation

[GitBox <gi...@apache.org>]

cloud-fan commented on a change in pull request #32242:
URL: https://github.com/apache/spark/pull/32242#discussion_r616445190



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/RowBasedHashMapGenerator.scala
##########
@@ -87,6 +87,18 @@ class RowBasedHashMapGenerator(
        |
        |    buckets = new int[numBuckets];
        |    java.util.Arrays.fill(buckets, -1);
+       |
+       |    // Register a cleanup task with TaskContext to ensure that memory is guaranteed to be
+       |    // freed at the end of the task. This is necessary to avoid memory leaks in when the
+       |    // downstream operator does not fully consume the aggregation map's output
+       |    // (e.g. aggregate followed by limit).
+       |    taskContext.addTaskCompletionListener(

Review comment:
       Can we do it outside of the fast hash map? Then we can apply it to both the row-based and vectorized fast hash map.




-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org