sunchao commented on code in PR #5493:
URL: https://github.com/apache/datafusion-comet/pull/5493#discussion_r3900888272
##########
spark/src/main/java/org/apache/spark/shuffle/comet/CometBoundedShuffleMemoryAllocator.java:
##########
@@ -75,6 +98,8 @@ public final class CometBoundedShuffleMemoryAllocator extends
CometShuffleMemory
this.pageSize = pageSize;
this.totalMemory =
CometSparkSessionExtensions$.MODULE$.getCometShuffleMemorySize(conf,
SQLConf.get());
+ this.memoryWaitTimeoutMs =
+ (long)
CometConf$.MODULE$.COMET_SHUFFLE_JVM_MEMORY_WAIT_TIMEOUT().get();
Review Comment:
[P2] Read the wait timeout from the task that is waiting
The per-task spill registry is a useful improvement. Could you resolve this
timeout once per `allocateBlocking` call, or explicitly enforce
executor-startup scope? `getInstance()` keeps this allocator in a static
`INSTANCE`, but this getter reads the initializing task's `SQLConf`. Later
tasks can carry a new `spark.comet.shuffle.jvm.memoryWaitTimeout` through
session settings and task properties, yet their deadline still uses the first
value.
For example, on a reused executor with the JVM/on-heap bypass path enabled
(`spark.comet.exec.onHeap.enabled=true` for plugin setup), initializing the
pool under `1s` and then changing the session setting to `5m` leaves a later
first-page wait using the one-second deadline. If another nonwaiting task holds
the needed memory past that deadline, a request that fits the pool can raise
OOM despite that task's longer configured wait. Reversing the values also
ignores a requested short cap. A regression test reusing the same allocator
across two task settings would cover this.
--
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]
