marin-ma commented on PR #12471:
URL: https://github.com/apache/gluten/pull/12471#issuecomment-5326118304

   @ReemaAlzaid Apologise for missing your previous comment. 
   
   Here's my understanding and some suggestions:
   
   The first issue `1. Broadcast joins built from an empty stream` can be 
workaround by setting 
`spark.gluten.velox.buildHashTableOncePerExecutor.enabled=false` because Velox 
Cudf doesn't support this feature. This PR disable this optimisation in the 
code so we don't need to explicitly disable the bhj optimisation for GPU 
workloads, and I agree with this change.
   
   The second issue `2. GPU vectors crashing on host reads` has occurred in the 
previous test but now it cannot be reproduced, while this PR still keep the 
changes to serve as a temporary remedy in case the CI failure in the future. If 
so, I do not agree with this change. The major reason is that it still does 
have the down side to burry some real issues by silencing the conversion, 
making the real issue hard to trace. In terms of GPU CI, currently we only have 
the CI job to test the GPU compilation, and there's no test running. If we can 
have the CI to run GPU testing in the future and once it's broken and needs fix 
from Velox, we can temporary disable the GPU CI rather than switching on this 
conversion in the code.


-- 
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]

Reply via email to