viirya commented on code in PR #380:
URL: https://github.com/apache/datafusion-comet/pull/380#discussion_r1591281558
##########
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:
Thanks @advancedxy. I debugged this issue but didn't find a quick fix so
decided to disable it temporarily.
I took a look at your branch. The Java allocator instance will report the
memory leak when getting closed if it has allocated memory size is larger then
zero. So as you did, if we find there is non-zero number (`getAllocatedMemory`
> 0), we don't close the allocator, it won't report that.
However, I'm not sure if it is correct fix and if we will ignore real memory
leak. Maybe it is a false positive one. But If it is real memory leak and we
ignore it, it will be a potential issue.
--
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]
