-
Notifications
You must be signed in to change notification settings - Fork 972
Support stage level schedule for final write stage #4574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
4a19fec to
213101b
Compare
...spark/kyuubi-extension-spark-common/src/main/scala/org/apache/kyuubi/sql/KyuubiSQLConf.scala
Outdated
Show resolved
Hide resolved
213101b to
f5bfd6b
Compare
Codecov Report
@@ Coverage Diff @@
## master #4574 +/- ##
============================================
+ Coverage 53.32% 53.36% +0.03%
Complexity 13 13
============================================
Files 573 577 +4
Lines 31501 31589 +88
Branches 4239 4245 +6
============================================
+ Hits 16797 16856 +59
- Misses 13128 13147 +19
- Partials 1576 1586 +10
... and 22 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
2f20ed4 to
93de3c5
Compare
...k/kyuubi-extension-spark-3-3/src/main/scala/org/apache/spark/CustomResourceProfileExec.scala
Outdated
Show resolved
Hide resolved
ec17f63 to
5ce2c5d
Compare
|
It is really hard to add some tests, so I test all of this manully. cc @yaooqinn @pan3793 @bowenliang123 @cfmcgrady if you have some other idea about this. |
|
Is there any description available for the manual test results other than the screenshots? I don't quite understand the screenshots showing consequence related to the purposed features. |
|
@bowenliang123 I have added some tags, ideally, the screenshots show that:
|
|
LGTM. Thx for the supplement. |
2 similar comments
|
LGTM. Thx for the supplement. |
|
LGTM. Thx for the supplement. |
5ce2c5d to
2225bec
Compare
66751d6 to
0b57d58
Compare
|
please separate this PR to small ones |
|
I seperate thie pr into two parts: 1. kill executor, 2. inject custom resource profile. here is the pr #4592 for kill executors |
Why are the changes needed?
This pr adds a new rule
FinalStageResourceManagerto inject custom reousrce profile for the final write stage.It nowprovide two features:
Kill redundant executors
We first get the final stage partition which is the actually required cores, then kill the redundant executors. The priority of kill executors follow:
Custom resource profile
We can specify the custom executor resource for final write stage. It now supports:
The reason why add this feature is that, if the previous stage contains lots executors but final stage has less, then the tasks of final stage would be scheduled randomly in all exists executors which may cause resource waste. e.g., each executor only run 1 or 2 tasks but holds 4 or 5 cores.
How was this patch tested?
test manually