You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@zhouyuan I’d like to pick this up. I think we can drop spark.gluten.velox.buildHashTableOncePerExecutor.enabled and the per-task branches it guarded, keeping the per-task build only when a benchmark dump is requested since the micro benchmark needs to replay the build side.
Does that match what you had in mind? Please let me know, thanks!
This topic came up during a discussion in a Gluten talk at Community Over Code Glasgow 2026. My understanding is that the broadcast hash join executor-side build cache option is mainly used for debugging and testing purposes, rather than in typical production scenarios.
If removing this option isn't straightforward or there are reasons to keep it, could we consider improving the documentation instead? It would be helpful to provide more details about its purpose, intended use cases, and when it might be useful to enable or disable it. since the shared BHJ map among executor threads are already be stressed
Thanks @zhouyuan. The removal turned out to be straightforward, so I’ve opened #13610 and we can judge from the diff. Two places used the “off” mode: VeloxBroadcastBuildOnceBenchmark, which only existed to compare the two modes, and DynamicOffHeapSizingSuite which disabled it with a note that the native build needs off-heap memory.
The PR removes the benchmark and lets that suite build once per executor like everything else. The per-task build stays only for whole-stage dumping which the micro benchmark relies on. CI is green too.
@JkSelf since you added the option, is there a debugging scenario where you still want to force a per-task build? If so, keeping and documenting the option is the easy fallback. Please let know, thank you!
We maintain the HashBuild logic in gluten cpp to pre-build the hash table. So it can easily cause our gluten unit tests failing if upstream velox change the logic of HashBuild in our daily rebase. And this option can make us fallback to original code path and not breaking the rebase work.
Currently i am working on removing this HashBuild logic in gluten by running a fake HashJoin job to build hash table in gluten side in #13216. And we can consider to remove this option after #13216 merged.
Thanks @JkSelf, that makes sense. I’ll convert this to a draft and rebase it once #13216 lands. In the meantime, per @zhouyuan’s suggestion, I’d like to improve the option’s description so it says what it is for which is an escape hatch for the daily Velox rebase when upstream changes the hash-build logic Gluten mirrors, with a note that it goes away after #13216. Happy to do that as a small follow-up unless you’d rather fold it into #13216.
Yes, just the description before #13216. Today the config string says what the option does but not why anyone would turn it off, which is the question @zhouyuan raised above. I’d expand it to say it is an escape hatch for the daily Velox rebase when upstream changes the hash-build logic Gluten mirrors and that it is slated for removal once #13216 gets merged.
If you’d rather not change it before #13216, I’m fine leaving it and just rebasing this PR afterwards. Please let know, thanks!
Activity
[-]Remove executor build cache option[/-][+][VL] Remove broadcast hash join executor build cache option[/+]anshulbaliga7 commentedon Nov 8, 2026
@zhouyuan I’d like to pick this up. I think we can drop
spark.gluten.velox.buildHashTableOncePerExecutor.enabledand the per-task branches it guarded, keeping the per-task build only when a benchmark dump is requested since the micro benchmark needs to replay the build side.Does that match what you had in mind? Please let me know, thanks!
zhouyuan commentedon Nov 9, 2026
@anshulbaliga7 Thanks for picking this up!
This topic came up during a discussion in a Gluten talk at Community Over Code Glasgow 2026. My understanding is that the broadcast hash join executor-side build cache option is mainly used for debugging and testing purposes, rather than in typical production scenarios.
If removing this option isn't straightforward or there are reasons to keep it, could we consider improving the documentation instead? It would be helpful to provide more details about its purpose, intended use cases, and when it might be useful to enable or disable it. since the shared BHJ map among executor threads are already be stressed
Cc: @JkSelf
anshulbaliga7 commentedon Nov 10, 2026
Thanks @zhouyuan. The removal turned out to be straightforward, so I’ve opened #13610 and we can judge from the diff. Two places used the “off” mode:
VeloxBroadcastBuildOnceBenchmark, which only existed to compare the two modes, andDynamicOffHeapSizingSuitewhich disabled it with a note that the native build needs off-heap memory.The PR removes the benchmark and lets that suite build once per executor like everything else. The per-task build stays only for whole-stage dumping which the micro benchmark relies on. CI is green too.
@JkSelf since you added the option, is there a debugging scenario where you still want to force a per-task build? If so, keeping and documenting the option is the easy fallback. Please let know, thank you!
JkSelf commentedon Nov 11, 2026
@anshulbaliga7 Thanks for your great working.
We maintain the HashBuild logic in gluten cpp to pre-build the hash table. So it can easily cause our gluten unit tests failing if upstream velox change the logic of HashBuild in our daily rebase. And this option can make us fallback to original code path and not breaking the rebase work.
Currently i am working on removing this HashBuild logic in gluten by running a fake HashJoin job to build hash table in gluten side in #13216. And we can consider to remove this option after #13216 merged.
anshulbaliga7 commentedon Nov 11, 2026
Thanks @JkSelf, that makes sense. I’ll convert this to a draft and rebase it once #13216 lands. In the meantime, per @zhouyuan’s suggestion, I’d like to improve the option’s description so it says what it is for which is an escape hatch for the daily Velox rebase when upstream changes the hash-build logic Gluten mirrors, with a note that it goes away after #13216. Happy to do that as a small follow-up unless you’d rather fold it into #13216.
Please let know, thank you!
JkSelf commentedon Nov 12, 2026
@anshulbaliga7 Do you mean to just improve the option description before #13216?
anshulbaliga7 commentedon Nov 12, 2026
Yes, just the description before #13216. Today the config string says what the option does but not why anyone would turn it off, which is the question @zhouyuan raised above. I’d expand it to say it is an escape hatch for the daily Velox rebase when upstream changes the hash-build logic Gluten mirrors and that it is slated for removal once #13216 gets merged.
If you’d rather not change it before #13216, I’m fine leaving it and just rebasing this PR afterwards. Please let know, thanks!