advancedxy commented on code in PR #380:
URL: https://github.com/apache/datafusion-comet/pull/380#discussion_r1591304514
##########
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:
> Maybe it is a false positive one. But If it is real memory leak and we
ignore it, it will be a potential issue.
Ah, yeah. I’m not 100 percent sure that the memory leak report is a false
positive as I haven’t verified at the native side with jvm running(it might be
quite tricky). Based on previous experience, the allocator could be closed
without failure at task completion though.
😂😂, it comes back to our previous conclusion that we may need to bridge the
java side with arrow-rs instead of arrow-java in the long-term. The allocator
API in the arrow-java is easy to misuse.
--
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]
