[WIP][EXTENSION] Inject Rule to fast fail gluten app if there are too much un-support operator #5321
Conversation
…ere are too much un-support operator
Codecov Report
@@ Coverage Diff @@
## master #5321 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 590 588 -2
Lines 33436 33399 -37
Branches 4422 4387 -35
======================================
+ Misses 33436 33399 -37 see 11 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
cc @ulysses-you |
zwangsheng
changed the title
| _: Bin | | ||
| _: Contains | | ||
| _: StartsWith | | ||
| _: EndsWith | |
There was a problem hiding this comment.
it seems Contains, StartsWith, EndsWith are supported
There was a problem hiding this comment.
Thanks, changed this list according to black list in
https://github.com/oap-project/gluten/blob/ad532a7b34cef1deeb6f3ae10f3ecc741c8875b9/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc#L38C1-L55
| "intended exposing to end users, it may be removed in anytime.") | ||
| .version("1.8.0") | ||
| .intConf | ||
| .createWithDefault(5) |
There was a problem hiding this comment.
let's use Int.MaxValue to avoid breaking something
| override def apply(plan: SparkPlan): SparkPlan = { | ||
| val count = plan.collect { | ||
| case p: FileSourceScanExec | ||
| if !p.relation.fileFormat.isInstanceOf[ParquetFileFormat] => |
| _: BroadcastNestedLoopJoinExec => | ||
| true | ||
| case p: SparkPlan | ||
| if p.expressions.exists(e => |
There was a problem hiding this comment.
if p.expressions.exists(_.exists {
...
})
| .createWithDefault(Int.MaxValue) | ||
|
|
||
| val GLUTEN_NON_SUPPORT_OPERATOR_LIST = | ||
| buildConf("spark.sql.gluten.nonSupportOperatorList") |
There was a problem hiding this comment.
how about naming spark.sql.gluten.fastFallbackOperators? and as an open source project, we'd better make the default value match a released version instead of the current main branch of Gluten
There was a problem hiding this comment.
+1, let's remove the default value and mark it as optional.
| } | ||
|
|
||
| object GlutenPlanAnalysis extends Rule[SparkPlan] { | ||
| private val nonSupportedOperatorList = conf.getConf(GLUTEN_NON_SUPPORT_OPERATOR_LIST) |
There was a problem hiding this comment.
please move this inside apply, so we can change this config at runtime
|
thanks @ulysses-you @pan3793 , make some changes:
|
4 participants
Why are the changes needed?
Fast fail if there are too much un-support operators in spark plan when enabled gluten.
Too much un-support operators when enabled gluten may significantly reduce the performance, so we want to notify user by fast fail this application.
If the user can tolerate this kind of performance penalty, you can increase the value of the
spark.sql.gluten.fallbackOperatorThreshold.See more gluten un-support operators in Velox Backend Support Progress
Note:
How was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before make a pull request
Was this patch authored or co-authored using generative AI tooling?
No