scheduler: DesiredCanaries can be set on every pass safely #8456
Conversation
This file contains 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
The field name `Deployment.TaskGroups` contains a map of `DeploymentState`, which makes it a little harder to follow state updates when combined with inconsistent naming conventions, particularly when we also have the state store or actual `TaskGroup`s in scope. This changeset changes all uses to `dstate` so as not to be confused with actual TaskGroups.
The reconcile loop sets `DeploymentState.DesiredCanaries` only on the first pass through the loop and if the job is not paused/pending. In MRD, deployments will make one pass though the loop while "pending", and were not ever getting `DesiredCanaries` set. We can't set it in the initial `DeploymentState` constructor because the first pass through setting up canaries expects it's not there yet. However, this value is static for a given version of a job because it's coming from the update stanza, so it's safe to re-assign the value on subsequent passes.
notnoop
approved these changes
Jul 20, 2020
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
The reconcile loop sets
DeploymentState.DesiredCanariesonly on the first pass through the loop and if the job is not paused/pending. In MRD, deployments will make one pass though the loop while "pending", and were not ever gettingDesiredCanariesset. We can't set it in the initialDeploymentStateconstructor because the first pass through setting up canaries expects it's not there yet. However, this value is static for a given version of a job because it's coming from the update stanza, so it's safe to re-assign the value on subsequent passes. This is fa63c8fIncludes a minor refactor to make it clear where we're accessing dstate. The field name
Deployment.TaskGroupscontains a map ofDeploymentState, which makes it a little harder to follow state updates when combined with inconsistent naming conventions, particularly when we also have the state store or actualTaskGroups in scope. Change all uses todstateso as not to be confused with actual TaskGroups. This is a separate commit 7552fff.