1145688 - [provisioner] hard kill nodes which are more than one hour older than workerType.lastModified

Status: RESOLVED INVALID

(Taskcluster :: Services, defect)

Description

[John Ford [:jhford] CET/CEST Berlin Time]

The workers should know to kill themselves after the workerType definition is updated, but as a fail-safe, we want to force kill workers which were created more than an hour before the lastModified time.

Comment 2

[John Ford [:jhford] CET/CEST Berlin Time]

Greg, what are your thoughts on this?  Do you think 1 hour is a good time frame or should it be longer?

Comment 3

[Greg Arndt [:garndt]]

I guess 1 hour is good for it to at least make sure an instance completes it's billing cycle that we paid for.  Perhaps maybe 90 minutes to provide wiggle room?

Comment 4

[Pete Moore [:pmoore][:pete]]

Hey John,

Do we still want to do this?

I remember we realised that lastModified also gets updated when e.g. we bump the maxCapacity, so we'd need to avoid machines shutting down when we bump the capacity.

In generic worker this is resolved now by having a "deploymentId" in the worker config, which we bump when we want older machines to die (typically when an AMI gets updated, or other key config changes). This way, machine termination is decided explicitly rather than implicitly, and makes it the active responsibility of the mechanism which updates the worker config.

In generic worker, the machines routinely query the provisioner to get the updated config, rather than the provisioner killing the workers - I kind of like this approach too, since it separates the responsibility of for provisioning instances with the provisioner, and the responsibility of terminating workers with the workers themselves (with the exception of the provisioner hard kill after 96 hours which still feels like a very sane fallback).

Comment 5

[John Ford [:jhford] CET/CEST Berlin Time]

You're right, we shouldn't trigger these hard kills for things which do not affect the running instance.  Whether that means not sending a pulse message, or not including those fields in a check of the current definition, I'm not sure.

I think that having an explicit thing which triggers a redeploy would be great in addition, but I do think there should be some sort of safety that forces redeployment when any worker-process-affecting portion of the worker-type definition changes.

Comment 6

[John Ford [:jhford] CET/CEST Berlin Time]

I think that comment 4 has a lot of merit.  I don't think this should be handled at the provisioner level, at least as things are implemented currently.