jackylee-ch commented on code in PR #13094:
URL: https://github.com/apache/gluten/pull/13094#discussion_r4240570233
##########
gluten-core/src/main/scala/org/apache/spark/memory/SparkMemoryUtil.scala:
##########
@@ -51,8 +53,21 @@ object SparkMemoryUtil {
// We assume storage memory can be fully transferred to execution memory so
far
def getCurrentAvailableOffHeapMemory: Long = {
val mm = SparkEnv.get.memoryManager
- val smp = smpField.get(mm).asInstanceOf[StorageMemoryPool]
- val emp = empField.get(mm).asInstanceOf[ExecutionMemoryPool]
+ // With dynamic off-heap sizing enabled, Gluten's global reservations are
+ // charged to the ON-heap pools (see GlobalOffHeapMemoryTarget), so the
+ // available figure must be read from the same pools. Read the flag the
same
+ // way GlobalOffHeapMemoryTarget.mode does, so the two cannot pick
different
+ // pool families.
+ val (smp, emp) =
+ if (GlutenCoreConfig.get.dynamicOffHeapSizingEnabled) {
Review Comment:
The comment says this reads the flag the same way
`GlobalOffHeapMemoryTarget.mode` does, but `mode` reads `SparkEnv.get.conf`
while `GlutenCoreConfig.get` resolves the active/thread-local `SQLConf`. On an
executor shuffle-writer thread with no active session, is this static
`buildStaticConf` flag guaranteed to be in `SQLConf.get`? If it falls back to
`false` there, we'd read the off-heap pools while `mode` reserves on-heap — the
wrong-pool-family case this PR fixes. Could we read it from `SparkEnv.get.conf`
here, identical to `mode`?
--
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]
