incubator-uniffle
incubator-uniffle copied to clipboard
[WIP] Introduce the general client conf for mr/spark
This PR is a proposal.
What changes were proposed in this pull request?
Introduce the general client conf for mr/spark
- Introduce the class of
RssClientConfto set the general configOptions for mr/spark - Introduce the class of
RssSparkClientConfto set the spark exclusiveconfigOptionsand to construct the RssConf from theSparkConf - Introduce the class of
RssMRClientConf
Why are the changes needed?
Now in mr/spark client conf, we don't obey the rule of uniffle configOptions. It causes the some problems as follows
- When introducing the client general conf, we have to write three times in
RssSparkConfig/RssClientConfig/RssMRConfig. Actually there is no need to leave dirty works for developers. - If when introducing a general method for MR/Spark and need to some configs from Rss client conf. In current implementation, we have to introduce extra class for method's params. If we having the general RssClientConf, it will benifit more.
Does this PR introduce any user-facing change?
No
How was this patch tested?
UTs.
Could u help check this proposal for mr/spark client conf? This POC only does the basic implementation. If this is OK, I will go ahead. @jerqi
The proposal is OK for me.
Codecov Report
Merging #200 (a6c0cc8) into master (331ebb2) will decrease coverage by
0.37%. The diff coverage is0.00%.
@@ Coverage Diff @@
## master #200 +/- ##
============================================
- Coverage 58.41% 58.04% -0.38%
+ Complexity 1273 1272 -1
============================================
Files 158 160 +2
Lines 8448 8497 +49
Branches 784 787 +3
============================================
- Hits 4935 4932 -3
- Misses 3260 3311 +51
- Partials 253 254 +1
| Impacted Files | Coverage Δ | |
|---|---|---|
| ...a/org/apache/spark/shuffle/RssSparkClientConf.java | 0.00% <0.00%> (ø) |
|
| ...rg/apache/uniffle/common/config/RssClientConf.java | 0.00% <0.00%> (ø) |
|
| ...org/apache/uniffle/server/ShuffleFlushManager.java | 76.66% <0.00%> (-1.67%) |
:arrow_down: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
Like discussion at dev mail list, we will freeze the code and cut 0.6 version branch in September 15, we will not merge this pr before I cut 0.6 version branch, are you ok?