[Improvement] Introduce config to customize assignment server numbers in client side #100
Conversation
|
Ping @jerqi |
Codecov Report
@@ Coverage Diff @@
## master #100 +/- ##
============================================
+ Coverage 56.34% 56.60% +0.25%
- Complexity 1175 1189 +14
============================================
Files 149 149
Lines 7977 8029 +52
Branches 765 769 +4
============================================
+ Hits 4495 4545 +50
- Misses 3239 3242 +3
+ Partials 243 242 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. |
|
How to rerun the CI @jerqi |
Close and reopen pull request. |
| @@ -65,4 +65,8 @@ public class RssClientConfig { | |||
| public static final int RSS_ACCESS_TIMEOUT_MS_DEFAULT_VALUE = 10000; | |||
| public static final String RSS_DYNAMIC_CLIENT_CONF_ENABLED = "rss.dynamicClientConf.enabled"; | |||
| public static final boolean RSS_DYNAMIC_CLIENT_CONF_ENABLED_DEFAULT_VALUE = true; | |||
|
|
|||
| public static final String RSS_CLIENT_ASSIGNMENT_SHUFFLE_SERVER_NUMBER = | |||
| "rss.client.assignment.shuffle.server.number"; | |||
There was a problem hiding this comment.
rss.client.assignment.shuffle.node.max
Could we use a similar style name with the coordinator's configuration?
|
Updated @jerqi |
|
@zuston Thanks for the contribution, it's flexible for different kind of jobs. And please update |
It should be a compatible feature, we don't need update the |
ok, it's only updated for coordinator. |
docs/client_guide.md
Outdated
| @@ -88,6 +88,7 @@ These configurations are shared by all types of clients. | |||
| |<client_type>.rss.client.send.threadPool.size|5|The thread size for send shuffle data to shuffle server| | |||
| |<client_type>.rss.client.assignment.tags|-|The comma-separated list of tags for deciding assignment shuffle servers. Notice that the SHUFFLE_SERVER_VERSION will always as the assignment tag whether this conf is set or not| | |||
| |<client_type>.rss.client.data.commit.pool.size|The number of assigned shuffle servers|The thread size for sending commit to shuffle servers| | |||
| |<client_type>.rss.client.assignment.shuffle.nodes.max|-1|The number of required assignment shuffle servers. If it is less than 0 or greater than the coordinator's config of "rss.coordinator.shuffle.nodes.max", it will use the size of "rss.coordinator.shuffle.nodes.max" default| | |||
There was a problem hiding this comment.
When the value is equal to 0, what will happen?
There was a problem hiding this comment.
If it is less than 0 or equal to 0 or greater than the coordinator's config of "rss.coordinator.shuffle.nodes.max", it will use the size of "rss.coordinator.shuffle.nodes.max" default.
I missed the condition of ==0.
| * case1: user specify the illegal shuffle node num(<0) | ||
| * it will use the default shuffle nodes num when having enough servers. | ||
| */ | ||
| PartitionRangeAssignment pra = strategy.assign(100, 10, 1, serverTags, -1); |
There was a problem hiding this comment.
Could you add a case when assignShuffleServer is equal to 0?
What changes were proposed in this pull request?
[Improvement] Introduce config to customize assignment server numbers in client side.
Changelog
<client_type>.rss.client.assignment.shuffle.nodes.maxWhy are the changes needed?
Now the assignment number specified by coordinator's conf of rss.coordinator.shuffle.nodes.max. But i think it's not suitable for all spark jobs.
We should introduce new config to let client specify the assignment server number. rss.coordinator.shuffle.nodes.max should be as a max limitation of clients' number.
Does this PR introduce any user-facing change?
YES.
How was this patch tested?
UT.