Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[FEATURE] Support dynamic client conf for creating ShuffleWriteClient #1554

Closed
3 tasks done
EnricoMi opened this issue Mar 1, 2024 · 0 comments
Closed
3 tasks done

[FEATURE] Support dynamic client conf for creating ShuffleWriteClient #1554

EnricoMi opened this issue Mar 1, 2024 · 0 comments
Assignees
@EnricoMi

Comments

@EnricoMi
Copy link
Contributor

EnricoMi commented Mar 1, 2024 •
edited
Loading

Code of Conduct

Search before asking

  • I have searched in the issues and found no similar issues.

Describe the feature

Dynamic client configuration should support configuring the ShuffleWriteClient of shuffle manager that fetches the config.

Motivation

Providing cluster-wide defaults via coordinators to configure the ShuffleWriteClient has operational benefits.

One use case is to have coordinators provide a BlockId layout configuration for clients (#1528).

Describe the solution

Currently, the dynamic client configuration cannot be used to configure the ShuffleWriteClient because the ShuffleWriteClient is used to fetch the dynamic client configuration, so the client has to be created first.

These solutions exist:

  1. Create another ShuffleWriteClient after dynamic client config has been fetched.
  2. Have RssShuffleManager create its own set of CoordinatorClients to fetch the dynamic client conf, then update the client configuration to finally create the ShuffleWriteClient.
  3. Provide the updated RssConfig to ShuffleWriteClient, to re-initialize from it.

Additional context

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!
This was referenced Mar 3, 2024
Closed
Merged
zuston pushed a commit that referenced this issue Mar 14, 2024
@EnricoMi
[#1554] feat(spark): Fetch dynamic client conf as early as possible (#…
de50442 
…1557)

### What changes were proposed in this pull request?
Fetches dynamic client config as early as possible, to be able to use dynamic client config to create the shuffle client with updated config.

### Why are the changes needed?
Providing config or the shuffle client via coordinator is operationally useful as cluster-wide settings can be deployed through the cluster and changed over time. Clients and apps do not need to change configs.

Fixes Spark part of #1554

### Does this PR introduce _any_ user-facing change?
More configs can be provided via coordinators.

### How was this patch tested?
Existing and [follow-up](https://github.com/apache/incubator-uniffle/pull/1528/files#diff-ea644edb1c0bf0e80f9a960adbc1615c99cb6a3a0d5fe24f788307f1daf22f46R127-R131) unit tests.
@zuston zuston closed this as completed Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@EnricoMi EnricoMi

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@zuston @EnricoMi