You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "jinchengchenghh (via GitHub)" <gi...@apache.org> on 2025/10/14 19:36:29 UTC
[PR] [GLUTEN-8851] Validate the plan before execution [incubator-gluten]
jinchengchenghh opened a new pull request, #10889:
URL: https://github.com/apache/incubator-gluten/pull/10889
Initializing task in Task constructor has been merged, so we can get the operator information from task, if validate successfully, set the whole stage transformer cudf tag to true to enable session config kCudfEnabled to true, and set the children operator CUDF tag to true to update the name in Spark UI with "Cudf".
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8851][VL] cuDF: Validate the plan before execution [incubator-gluten]
Posted by "jinchengchenghh (via GitHub)" <gi...@apache.org>.
jinchengchenghh commented on code in PR #10889:
URL: https://github.com/apache/incubator-gluten/pull/10889#discussion_r2444947419
##########
backends-velox/src/main/scala/org/apache/gluten/extension/CudfNodeValidationRule.scala:
##########
@@ -37,7 +38,22 @@ case class CudfNodeValidationRule(glutenConf: GlutenConfig) extends Rule[SparkPl
case _: LeafTransformSupport => true
case _ => false
}.isDefined
- transformer.setTagValue(CudfTag.CudfTag, !hasLeaf)
+ if (!hasLeaf && VeloxConfig.get.cudfEnableValidation) {
+ if (
+ VeloxCudfPlanValidatorJniWrapper.validate(
+ transformer.substraitPlan.toProtobuf.toByteArray)
+ ) {
+ transformer.foreach {
+ case _: LeafTransformSupport =>
+ case t: TransformSupport =>
+ t.setTagValue(CudfTag.CudfTag, true)
+ case _ =>
+ }
+ transformer.setTagValue(CudfTag.CudfTag, true)
+ }
+ } else {
+ transformer.setTagValue(CudfTag.CudfTag, !hasLeaf)
+ }
Review Comment:
Will not check if the pan is fully offloaded to GPU, may introduce extra cudf to velox format conversion. This is not a suggested behavior.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8851][VL] cuDF: Validate the plan before execution [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on code in PR #10889:
URL: https://github.com/apache/incubator-gluten/pull/10889#discussion_r2444870601
##########
backends-velox/src/main/scala/org/apache/gluten/extension/CudfNodeValidationRule.scala:
##########
@@ -37,7 +38,22 @@ case class CudfNodeValidationRule(glutenConf: GlutenConfig) extends Rule[SparkPl
case _: LeafTransformSupport => true
case _ => false
}.isDefined
- transformer.setTagValue(CudfTag.CudfTag, !hasLeaf)
+ if (!hasLeaf && VeloxConfig.get.cudfEnableValidation) {
+ if (
+ VeloxCudfPlanValidatorJniWrapper.validate(
+ transformer.substraitPlan.toProtobuf.toByteArray)
+ ) {
+ transformer.foreach {
+ case _: LeafTransformSupport =>
+ case t: TransformSupport =>
+ t.setTagValue(CudfTag.CudfTag, true)
+ case _ =>
+ }
+ transformer.setTagValue(CudfTag.CudfTag, true)
+ }
+ } else {
+ transformer.setTagValue(CudfTag.CudfTag, !hasLeaf)
+ }
Review Comment:
A dumb question, after this PR, what happens if user sets `spark.gluten.sql.columnar.backend.velox.cudf.enableValidation=false` but the plan is actually invalid for cuDF?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8851][VL] cuDF: Validate the plan before execution [incubator-gluten]
Posted by "jinchengchenghh (via GitHub)" <gi...@apache.org>.
jinchengchenghh merged PR #10889:
URL: https://github.com/apache/incubator-gluten/pull/10889
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org