orchestrator: Flag tasks that shouldn't be restarted#2327
Closed
aaronlehmann wants to merge 1 commit intomoby:masterfrom
Closed
orchestrator: Flag tasks that shouldn't be restarted#2327aaronlehmann wants to merge 1 commit intomoby:masterfrom
aaronlehmann wants to merge 1 commit intomoby:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2327 +/- ##
==========================================
- Coverage 60.31% 60.24% -0.07%
==========================================
Files 128 128
Lines 26002 26032 +30
==========================================
Hits 15683 15683
- Misses 8929 8959 +30
Partials 1390 1390 |
Contributor
|
High-level seems like a decent solution. Following #2290 I don't see a better alternative, but I'm also no longer super familiar with this portion of the codebase. |
cyli
reviewed
Jul 24, 2017
|
|
||
| service.UpdateStatus.State = api.UpdateStatus_ROLLBACK_STARTED | ||
| service.UpdateStatus.Message = message | ||
| err = batch.Update(func(tx store.Tx) error { |
Contributor
There was a problem hiding this comment.
Non-blocking nitpick: maybe just return batch.Update here?
cyli
approved these changes
Jul 24, 2017
Contributor
cyli
left a comment
There was a problem hiding this comment.
As someone not very familiar with the intricacies of the orchestrator, reaper, updater, etc., this solution makes more sense to me and is easier to understand.
It seems fine to me to rely on the task history, for --task-history-limit 0 to be a best effort sort of thing (maybe if that's specified, we can just hide the task history in the API but actually still keep it around)?
Previously, restart conditions other than "OnAny" were honored on a best-effort basis. A service-level reconciliation, for example after a leader election, would see that not enough tasks were running, and start replacement tasks regardless of the restart policy. This limited the usefulness of the other restart conditions. This change adds a DontRestart flag to Task. It can be set by the restart supervisor when it shuts down a task and decides not to start a replacement task. The orchestrators look for the presence of this flag and honor it when doing service-level reconciliation. If the flag is set, the dead task is passed to the updater along with the running tasks, so the updater can start a replacement if and only if the service definition has changed relative to the dead task. The task reaper has been modified so it will never delete the last task in a slot, if that task has the DontRestart flag set. Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
2d279a2 to
43bbe90
Compare
This was referenced Jul 24, 2017
Collaborator
Author
|
Superseded by #2332 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, restart conditions other than "OnAny" were honored on a
best-effort basis. A service-level reconciliation, for example after a
leader election, would see that not enough tasks were running, and start
replacement tasks regardless of the restart policy. This limited the
usefulness of the other restart conditions.
This change adds a
DontRestartflag toTask. It can be set by therestart supervisor when it shuts down a task and decides not to start a
replacement task. The orchestrators look for the presence of this flag
and honor it when doing service-level reconciliation. If the flag is
set, the dead task is passed to the updater along with the running
tasks, so the updater can start a replacement if and only if the service
definition has changed relative to the dead task.
The task reaper has been modified so it will never delete the last task
in a slot, if that task has the
DontRestartflag set.Fixes #932
Counterproposal to #2290, informed by corner cases encountered there
cc @cyli @aluzzardi