-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
🌱 Introduce a new runnable group for basic servers of the manager #2337
🌱 Introduce a new runnable group for basic servers of the manager #2337
Conversation
cc @sbueringer plz double check if this works under your case, thanks |
I'll take a look soon, but I would delay merging it until after the v0.15 release (just because we don't strictly need it ASAP + it is a risk directly before the release) |
Is there a specific problem we're trying to solve to have the servers in a runnable group? |
Yep, if we don't have them in a specific runnable group, they would be added to the In a word, we need to have the servers in a specific runnable group which would be started first (at least before the |
I think Vince might be more asking why should it be a runnable vs the current state. To answer that, @vincepri this goes back to #1943 (comment) |
5f9b66d
to
ad70da2
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vincepri, zqzten The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/lgtm |
This PR introduces a new runnable group for basic servers (health probes, metrics and pprof server) of the manager and make it start before any other runnables. This can resolve the cyclic dependency issue after we changing these servers to runnables.