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.

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

[FEATURE] Support the hot reload of coordinator's configuration rss.coordinator.shuffle.nodes.max #410

Closed
3 tasks done
jerqi opened this issue Dec 13, 2022 · 3 comments · Fixed by #572
Closed
3 tasks done
Assignees

Comments

@jerqi
Copy link
Contributor

jerqi commented Dec 13, 2022

Code of Conduct

Search before asking

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

Describe the feature

rss.coordinator.shuffle.nodes.max don't support hot reload. It's not friendly for operation.

Motivation

It will be more friendly for operation

Describe the solution

No response

Additional context

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!
@advancedxy
Copy link
Contributor

Maybe we can extend this issue title to a bit more generic: support hot reload of Uniffle configurations? It can be used in both shuffle server and coordinator side, but mainly for coordinator.

Once uniffle supports deployments on K8S, it's common to hot-reload configurations backend by using ConfigMap.

@advancedxy
Copy link
Contributor

@jerqi Do you have time for this issue. I'd like this feature to be merged in our next release, such as 0.6.1 or 0.7.0

@jerqi
Copy link
Contributor Author

jerqi commented Feb 8, 2023

I will finish this issue this week.

@jerqi jerqi self-assigned this Feb 8, 2023
jerqi added a commit to jerqi/incubator-uniffle that referenced this issue Feb 9, 2023
jerqi added a commit that referenced this issue Feb 10, 2023
)

### What changes were proposed in this pull request?
I refer to Hadoop implement. I design a hot reload process. We support nodeMax at first.

### Why are the changes needed?
It's more convenient.

### Does this PR introduce _any_ user-facing change?
add `rss.reconfigure.interval.sec`.

### How was this patch tested?
UT

Co-authored-by: roryqi <roryqi@tencent.com>
Co-authored-by: Kaijie Chen <ckj@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants