Swarmkit: Rolling Updates: Failure Threshold

[aluzzardi]

Updated Issue

Currently, all it takes for a rolling update to "fail" is a single task.

This is somewhat flawed since in fairly large deployments it'll be fairly common to have a task failing for completely unrelated reasons.

Original Issue

In the current implementation of rolling updates, the updater will wait forever until a task moves to RUNNING. If it doesn't, that go-routine will be stuck almost forever. Almost, because in case of manager restart or failover, rolling updates will attempt again.

We need to define a better model. Perhaps:

• Time out while waiting for a task to become running
• Abort update after a threshold of failures is observed
• Rollback the update

[aaronlehmann]

I suppose this depends on health checks to give us a definition of a successful update. Waiting for a task to reach the RUNNING state doesn't accomplish much, because the task could die immediately afterwards, or be in a non-functional state.

[aluzzardi]

With the current health check specs, a task wouldn't move to RUNNING until health checks are green.

[aaronlehmann]

Then I think there's a lot of overlap between this and #488.

[aaronlehmann]

Proposal:

diff --git a/api/types.proto b/api/types.proto
index 2ffa305..74f770c 100644
--- a/api/types.proto
+++ b/api/types.proto
@@ -236,6 +236,15 @@ message UpdateConfig {

        // Amount of time between updates.
        uint64 delay = 2 [(gogoproto.customtype) = "time.Duration", (gogoproto.nullable) = false];
+
+       // TaskStartTimeout is the time after which a replacement task that
+       // does reach the RUNNING state is considered to have failed.
+       uint64 task_start_timeout = 3 [(gogoproto.customtype) = "time.Duration", (gogoproto.nullable) = false];
+
+       // MaxFailures is the threshold of failed task executions that will
+       // cause the update to be rolled back. If set to zero, the update will
+       // never be automatically rolled back.
+       uint64 max_failures = 4;
 }

 // TaskState enumerates the states that a task progresses through within an

[aaronlehmann]

Actually, I wonder if "Time out while waiting for a task to become running" is more of a general orchestrator concern than something specific to rolling updates. We could have the same issue with tasks getting wedged when doing initial orchestration for a service. Should we move the timeout to the service spec, and enforce it as part of orchestrator reconcillation?

[dongluochen]

I agree timeout should be in service spec, i.e., each service has its own start up duration. Initial deployment has a rollout failure issue. I think rolling update and initial deployment does share the same service start timeout.

[aluzzardi]

Agree as well - it's a general orchestrator thing rather than an update concern.

Regarding rolling updates, as discussed earlier, a possible solution to achieve that would be to add an extra field to Service, perhaps previous_spec.

When a rolling update fails (and here we need to define failure, such as a failure threshold), the updater could set spec to previous_spec, previous_spec to nil and exit. The reconciliation loop would kick in (since spec changed) and perform a rolling update the other way around and restore to the previous version. Since previous_spec is nil, it would not rollback the rollback in case of failure so we avoid an infinite back and forth.

On the RPC side, CreateService sets spec to the spec and previous_spec to nil. UpdateService sets previous_spec to the current spec, and spec to the request spec.

We also need to provide a policy to the user (e.g. on failure: continue, abort, rollback).

[nishanttotla]

There are a couple of questions I had regarding this issue:

1. Is it possible to interfere with rolling updates? For instance, if someone changes the number of instances while a rolling update is in progress, what is the expected behavior? While playing with it, I didn't run into any failures, but I want to be sure about it. How is this dealt with?
2. Do we need to validate rolling updates -- as in, what counts as a rolling update? Is any change in the configuration for tasks (as long it's the same service), which can be image version, changes in network/volume etc. counted for rolling update?

cc @aaronlehmann @dongluochen

[dongluochen]

Is it possible to interfere with rolling updates?

Yes. User should be able to pause/unpause update, cancel update. If an update is cancelled, it'd be in a dirty state where 2 versions of the service are in place.

For instance, if someone changes the number of instances while a rolling update is in progress, what is the expected behavior? While playing with it, I didn't run into any failures, but I want to be sure about it. How is this dealt with?

Scaling up might be easy as you don't compete on the same task. Scaling down might be a problem. We also need to think about autoscaling support and the potential of interference. There is an interesting scenario. A new version of service takes much more CPU so autoscaling is triggered.

Do we need to validate rolling updates -- as in, what counts as a rolling update? Is any change in the configuration for tasks (as long it's the same service), which can be image version, changes in network/volume etc. counted for rolling update?

I think change in task should be counted as rolling update. User don't want to wait can tune the parameters of update process.

[nishanttotla]

@aaronlehmann @dongluochen I think #1069 is a good use case for having some sort of rollback mechanism. Ties back to the idea of "validating" an update before going ahead with it. In that issue, all tasks would eventually be killed off in favor of new ones, none of which would ever run.

One way to approach this is that the orchestrator shouldn't have to worry about rollbacks and repairing state. This could be done entirely on the client side. That way, the orchestrator simply cares about enforcing a given state, and the client worries about ensuring that it's actually feasible and rolling back if not. The downside is that this puts a lot of responsibility on the client and probably breaks some abstractions that users might expect from our API. Thoughts?

[dongluochen]

@nishanttotla I think rolling update should be done on server side. Server can keep track of current status and carry out tasks even some managers fail. Client cannot do that.

[stevvooe]

@nishanttotla This is a 3 month old issue, filed before rolling updates were implemented. We need to revisit. This should probably be closed and we should address in another issue.

Server can keep track of current status and carry out tasks even some managers fail. Client cannot do that.

I disagree. This can be done completely client side. The orchestrator is going to have a very tough time deciding what a failed update is without extensive policy specification features.

Let's make sure what we have now is truly insufficient piling features on...

[aaronlehmann]

I think we should have some kind of health check awareness in rolling updates in 1.12.

@aluzzardi: Thoughts?

On Jun 28, 2016, at 20:13, Stephen Day [email protected] wrote:

@nishanttotla This is a 3 month old issue, filed before rolling updates were implemented. We need to revisit. This should probably be closed and we should address in another issue.

Server can keep track of current status and carry out tasks even some managers fail. Client cannot do that.

I disagree. This can be done completely client side. The orchestrator is going to have a very tough time deciding what a failed update is without extensive policy specification features.

Let's make sure what we have now is truly insufficient piling features on...

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[aluzzardi]

@aaronlehmann If the agent correctly enforces health checks, then we'd get that for free since a failed health check results in the task moving to FAILED.

Also, until the health checks succeed, the task should remain in STARTING

Again, that's the theory of the original design and that's how it should work

[nishanttotla]

I think that's right. If updated tasks are failing, they'll be moved to
FAILED. Although we could use it further to rollback if multiple tasks
start failing.
On Wed, Jun 29, 2016 at 18:48 Andrea Luzzardi [email protected]
wrote:

@aaronlehmann https://github.com/aaronlehmann If the agent correctly
enforces health checks, then we'd get that for free since a failed health
check results in the task moving to FAILED.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/docker/swarmkit/issues/486#issuecomment-229538533,
or mute the thread
https://github.com/notifications/unsubscribe/ABySmVV9qOPKXky4q8Cgkd2mAtPimVtYks5qQyBWgaJpZM4ITR8K
.

[aaronlehmann]

But there's nothing in rolling updates to abort or roll back based on FAILED tasks. I think we should add that.

On Jun 29, 2016, at 18:48, Andrea Luzzardi [email protected] wrote:

@aaronlehmann If the agent correctly enforces health checks, then we'd get that for free since a failed health check results in the task moving to FAILED.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[aluzzardi]

We don't have auto-rollback. I don't know if it's too late for 1.12 though.

[aaronlehmann]

I'm starting to work on auto-rollback. I think there's a decent chance we can do it for 1.12.

Here's what I have in mind:

• The service will have the previous version of the spec as well as the current version. The UpdateService controlapi handler will rotate the old version of the spec to the field containing the previous version if the spec is updated.
• The service spec will contain a threshold for how many tasks must end up in a running state, given as a fraction or percentage. I want to express it this way instead of a fraction or percentage of tasks which can fail, so that a value of 0 means "never rollback".
• During a rolling update, we watch all tasks that were created by the rolling update. When any of those fail, we increment a local counter that we compare against the threshold. At the beginning of the update, we initialize the counter to the number of failed tasks with the current version of the task spec, so that if there's a manager failover in the middle of the update, we're still counting correctly.
• When the number of failed tasks exceeds the threshold, replace the service spec with the "old" value and kick off another rolling update. The "old" value will now match the current value, so it will not be possible to oscillate between spec versions.
• The orchestrator won't know anything about health checks. All it cares about is whether containers reach the RUNNING state and stay there for the duration of the update. Once the update is over, automatic rollback can no longer be triggered.

Thoughts?

[nishanttotla]

Couple of minor clarifying questions:

The service spec will contain a threshold for how many tasks must end up in a running state

This is counted across tasks matching old and new spec, right?

When the number of failed tasks exceeds the threshold, replace the service spec with the "old" value and kick off another rolling update.

This threshold here is the threshold provided above (in the spec) subtracted from the total tasks, right?

[aaronlehmann]

This is counted across tasks matching old and new spec, right?

I should have specified. It would be the number of updated tasks that get to a running state, divided by the total number of tasks (new and old). For a replicated service, the denominator would be equivalent to the replica count (but we probably wouldn't implement it that way, to keep the updater generic).

This threshold here is the threshold provided above (in the spec) subtracted from the total tasks, right?

The threshold in the spec will be given as a fraction or percentage (haven't decided), but yes, the condition for rollback would essentially be FailedTasks >= (1-SuccessFraction)*TotalTasks.

[dongluochen]

This is counted across tasks matching old and new spec, right?

I think so. If a service is already failing above threshold before update, update should not start. This would keep user service up and running as much as possible.

[aaronlehmann]

If a service is already failing above threshold before update, update should not start.

I don't agree with this (or I misunderstand). That would make it impossible to recover from something like a bad image that causes all tasks to fail. I think the condition should be updated tasks that get to a running state, divided by total number of tasks (new and old).

[aluzzardi]

I think the behavior should be user configurable: ABORT, ROLLBACK, CONTINUE (the current behavior we have is CONTINUE).

I do also think that before implementing new features (I think of this as a "OnFailure" strategy or something) we should make sure what we have is stable since we're approaching the release.

I'd prefer if we started by writing integration tests for rolling updates before we add another feature.

[aaronlehmann]

I think the behavior should be user configurable: ABORT, ROLLBACK, CONTINUE (the current behavior we have is CONTINUE).

I agree we should provide this choice. But note that our current behavior isn't necessarily CONTINUE. If new tasks don't reach RUNNING, for example because of a bad image reference, the update will get stuck waiting for those tasks.

[aluzzardi]

@aaronlehmann That sounds like a regression. The update manager used to wait for >= RUNNING - if they failed, it would just move on with the update.

Which goes back to my point regarding integration tests. We got quite a lot of regressions during the past month or so

[aaronlehmann]

@aluzzardi: You're correct, I missed that while looking at the code.

[dongluochen]

I don't agree with this (or I misunderstand). That would make it impossible to recover from something like a bad image that causes all tasks to fail. I think the condition should be updated tasks that get to a running state, divided by total number of tasks (new and old).

User defines a total failing threshold. If at any time failing number is larger than threshold, it should ABORT, or ROLLBACK. This would prevent a bad image from failing all tasks. If a service does get over the failing threshold for any reason, user needs to change the failing threshold to recover.

An extreme case is user has a Paxos quorum of 3 nodes. If update on the first node fails, it should rollback on the first node. Trying to update the second node may fail service.

[aluzzardi]

@aaronlehmann The title is poorly phrased, but assuming the core of the problem was:

In the current implementation of rolling updates, the updater will wait forever until a task moves to RUNNING

Can this be closed down?

I'll open a rollback issue separately

[aaronlehmann]

There's useful discussion here - maybe we can change the title and description?

[aluzzardi]

@aaronlehmann Updated title and description. Sounds good?