You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We've tested the new feature implemented in #13144
Is it intended to resume supervisors after an overlord restart? It would be helpful to persist the idle state for supervisors so that they don't create new tasks when overlord restarts for example.
Motivation
We're running druid in k8s and overlord could restart anytime. With the idle feature, we could save a lot of middlemanagers from running. Though, when overlord is restarted all supervisors will resume and create tasks for each supervisor and our running middlemanagers will be overloaded.
From reading the code, as far as I can tell, the state is not persisted, and it isn't set back up on supervisor restart until inactiveAfterMillis has elapsed.
I can think of a couple options for addressing this. One is that we could treat idle like suspended, by putting it in the supervisor spec and persisting it. However this seems weird to me, since it would involve the supervisor needing to edit its own spec once it enters idle state, and typically supervisors don't edit their own specs in the metadata store. So this wouldn't be my preferred choice.
Another, IMO better approach is to adjust the logic for supervisor startup to get into the idle state immediately if that makes sense. Logic could be something like: if committed offsets match the current stream offsets, and no tasks are running, go into idle immediately. I think we check the current stream offsets once a minute, so that means the main risk is that we'll launch tasks up to a minute late in the case where the offsets did match, but then a new message comes in right after the supervisor starts up. But that should be rare, and if it does happen, it means messages are coming in quite infrequently. So it's probably fine to err on the side of staying idle for up to a minute.
this makes sense to me, but wouldn't the behavior you describe be the same as the current behavior during steady state of the supervisor?
if a message came in while the supervisor was idle and the overlord was restarting, computeTotalLag() == 0 would not be true and the supervisor would not be marked as idle when it first runs.
if it came in after the supervisor started up and was set as idle, that's pretty much the same time frame as the supervisor being idle, a message coming in, and then the supervisor starting a new task during steady state operation
Yeah, I think that's right, which is good. It means that supervisors being able to go idle immediately is actually even more OK than I thought 🙂
One implementation note btw: it looks like currently the latest offsets are not populated on supervisor startup. It happens in a separate reporting thread. So there'd have to be an explicit initial population of those offsets prior to starting up the reporting thread.
This issue has been marked as stale due to 280 days of inactivity.
It will be closed in 4 weeks if no further activity occurs. If this issue is still
relevant, please simply write any comment. Even if closed, you can still revive the
issue at any time or discuss it on the dev@druid.apache.org list.
Thank you for your contributions.
Activity
gianm commentedon Oct 24, 2023
From reading the code, as far as I can tell, the state is not persisted, and it isn't set back up on supervisor restart until
inactiveAfterMillishas elapsed.I can think of a couple options for addressing this. One is that we could treat
idlelikesuspended, by putting it in the supervisor spec and persisting it. However this seems weird to me, since it would involve the supervisor needing to edit its own spec once it enters idle state, and typically supervisors don't edit their own specs in the metadata store. So this wouldn't be my preferred choice.Another, IMO better approach is to adjust the logic for supervisor startup to get into the idle state immediately if that makes sense. Logic could be something like: if committed offsets match the current stream offsets, and no tasks are running, go into idle immediately. I think we check the current stream offsets once a minute, so that means the main risk is that we'll launch tasks up to a minute late in the case where the offsets did match, but then a new message comes in right after the supervisor starts up. But that should be rare, and if it does happen, it means messages are coming in quite infrequently. So it's probably fine to err on the side of staying idle for up to a minute.
Thoughts?
georgew5656 commentedon Oct 24, 2023
this makes sense to me, but wouldn't the behavior you describe be the same as the current behavior during steady state of the supervisor?
if a message came in while the supervisor was idle and the overlord was restarting, computeTotalLag() == 0 would not be true and the supervisor would not be marked as idle when it first runs.
if it came in after the supervisor started up and was set as idle, that's pretty much the same time frame as the supervisor being idle, a message coming in, and then the supervisor starting a new task during steady state operation
gianm commentedon Oct 24, 2023
Yeah, I think that's right, which is good. It means that supervisors being able to go idle immediately is actually even more OK than I thought 🙂
One implementation note btw: it looks like currently the latest offsets are not populated on supervisor startup. It happens in a separate reporting thread. So there'd have to be an explicit initial population of those offsets prior to starting up the reporting thread.
github-actions commentedon Jul 31, 2024
This issue has been marked as stale due to 280 days of inactivity.
It will be closed in 4 weeks if no further activity occurs. If this issue is still
relevant, please simply write any comment. Even if closed, you can still revive the
issue at any time or discuss it on the dev@druid.apache.org list.
Thank you for your contributions.
Check if supervisor could be idle on startup (apache#16844)
Check if supervisor could be idle on startup (apache#16844)