feat: Add configs to disable specified operator #850
Conversation
| createExecDisabledConfig(OPERATOR_BROADCAST_EXCHANGE, defaultValue = false) | ||
| val COMET_EXEC_HASH_JOIN_DISABLED: ConfigEntry[Boolean] = | ||
| createExecDisabledConfig(OPERATOR_HASH_JOIN, defaultValue = false) | ||
| val COMET_EXEC_SORT_MERGE_JOIN_DISABLED: ConfigEntry[Boolean] = |
There was a problem hiding this comment.
For example, I use this to test SortMergeJoin in #553.
There was a problem hiding this comment.
It seems strange to have both an enabled and disabled for each operator. What do we do if both are true or both are false?
In #840 I had suggested another approach, which I wasn't too happy with, and @parthchandra had added another suggestion in there
There was a problem hiding this comment.
In isCometOperatorEnabled, currently disabled configs have higher priority.
But I agree that it would make confusion.
Another proposal would be to disable the operator if enabled config is false even COMET_EXEC_ALL_OPERATOR_ENABLED is true.
There was a problem hiding this comment.
I.e., the enabled can be used to enable/disable specified operator.
There was a problem hiding this comment.
Hmm, it doesn't make sense too. These enabled configs are false by default. Which will disable these configs.
There was a problem hiding this comment.
Hmm, it doesn't make sense too. These
enabledconfigs are false by default. Which will disable these configs.
In PR I had changed them all to default to true, so we can disable by setting to false
There was a problem hiding this comment.
I had also removed the ALL option
There was a problem hiding this comment.
I had also removed the
ALLoption
You mean COMET_EXEC_ALL_OPERATOR_ENABLED?
There was a problem hiding this comment.
Okay, it makes sense then. Do I need to wait for your PR?
There was a problem hiding this comment.
I created a new PR with just the config change
#855
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #850 +/- ##
=============================================
+ Coverage 34.05% 55.34% +21.28%
+ Complexity 879 857 -22
=============================================
Files 112 109 -3
Lines 42959 10585 -32374
Branches 9488 2010 -7478
=============================================
- Hits 14629 5858 -8771
+ Misses 25330 3714 -21616
+ Partials 3000 1013 -1987 ☔ View full report in Codecov by Sentry. |
Which issue does this PR close?
Closes #.
Rationale for this change
In contrast to the configs enabling operators, sometimes we only want to disable one particular operator in Comet to check if it is the cause of error. For the use case, we need the configs used to disable specified operators.
What changes are included in this PR?
How are these changes tested?