LuciferYang commented on code in PR #12622:
URL: https://github.com/apache/gluten/pull/12622#discussion_r3646102573
##########
gluten-core/src/main/scala/org/apache/gluten/GlutenPlugin.scala:
##########
@@ -134,6 +135,15 @@ private object GlutenDriverPlugin extends Logging {
((onHeapSize - (300 * 1024 * 1024)) *
conf.getDouble(GlutenCoreConfig.DYNAMIC_OFFHEAP_SIZING_MEMORY_FRACTION.key,
0.6d)).toLong
} else {
+ // Untracked memory mode skips the off-heap size requirement in
checkOffHeapSettings, so
+ // the key may be absent here. Normalize it to 0 (mirroring the
dynamic-sizing branch
+ // above) so downstream readers that read spark.memory.offHeap.size
directly, e.g.
+ // VeloxListenerApi.onDriverStart, don't hit NoSuchElementException.
Normal mode always has
+ // the key set because checkOffHeapSettings enforced it, so this only
affects untracked
+ // mode without an explicit off-heap size.
+ if (!conf.contains(GlutenCoreConfig.SPARK_OFFHEAP_SIZE_KEY)) {
Review Comment:
It's still needed for untracked mode. You're right that we validate the
off-heap size in `checkOffHeapSettings`, but that path only runs in normal
mode: untracked mode returns early at L86-89 (the `COLUMNAR_MEMORY_UNTRACKED`
branch), before the L92-102 size check, so `spark.memory.offHeap.size` can stay
absent.
Repro: `spark.gluten.memory.untracked=true` with `spark.memory.offHeap.size`
unset and dynamic sizing off → `checkOffHeapSettings` returns at L89 → the
else-branch (main L136) calls the single-arg
`conf.getSizeAsBytes(SPARK_OFFHEAP_SIZE_KEY)`, which throws
`NoSuchElementException` because `SparkConf.get(String)` reads the raw settings
map and doesn't fall back to the ConfigEntry default. The added
`GlutenDriverPluginSuite` test "setPredefinedConfigs does not throw in
untracked mode without an off-heap size" fails without this change.
If it reads clearer, I can add a short comment at the untracked early-return
noting the size isn't validated there.
--
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]
