advancedxy commented on code in PR #380:
URL: https://github.com/apache/datafusion-comet/pull/380#discussion_r1591258340
##########
spark/src/main/scala/org/apache/comet/CometSparkSessionExtensions.scala:
##########
@@ -916,7 +916,10 @@ object CometSparkSessionExtensions extends Logging {
private[comet] def isCometShuffleEnabled(conf: SQLConf): Boolean =
COMET_EXEC_SHUFFLE_ENABLED.get(conf) &&
(conf.contains("spark.shuffle.manager") &&
conf.getConfString("spark.shuffle.manager") ==
- "org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager")
+ "org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager") &&
+ // TODO: AQE coalesce partitions feature causes Comet shuffle memory
leak.
Review Comment:
> But for now, I think it is better to disable it temporarily for the cases
we know it will cause some issues.
Ah, yeah. It makes sense to disable it first if it takes a lot of time and
resources to debug and fix later on.
The issue you described seems like a similar problem I encountered when
adding support for CometRowToColumnar in #206. So I just went ahead and did a
quick investigation based on your branch. It seems that we cannot close the
allocator prematurely as the record might still be used in the native side, see
these comments for more details:
https://github.com/apache/datafusion-comet/pull/206/files#diff-04037044481f9a656275a63ebb6a3a63badf866f19700d4a6909d2e17c8d7b72R37-R46
I also submit a new
commit(https://github.com/advancedxy/arrow-datafusion-comet/commit/48517ef2ff713f2f195539da71ea404041778466)
as a potential fix in my branch, hoping that helps you out. The test code is
just modified for demonstration purposes.
--
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]
