COM-231: Support worker set launch strategy in InvokeWorkersToRegisterTaskQueues #98
Conversation
justinschoeff
left a comment
justinschoeff
left a comment
There was a problem hiding this comment.
Do we want to add tests in a follow-on, I don't see coverage in activities_test currently? We do have coverage for it in integ tests though.
| // registerTaskQueueWorkerSetSize is the instance count a worker-set provider is | ||
| // scaled to so a single worker comes up and registers the task queue. The scaling | ||
| // algorithm resizes the set from there. | ||
| registerTaskQueueWorkerSetSize int32 = 1 |
There was a problem hiding this comment.
should this be seeding to initial_count instead of 1?
There was a problem hiding this comment.
Initial count may be 0
There was a problem hiding this comment.
ah you are correct, that is possible. I'm thinking through a scenario where the update to 1 goes unnoticed by the scaling logic when it reads the actual state as initial_count instead of 1 and skips performing a scaling action.
say a customer set it to initial_count, and we primed it down to 1. and the scaling algo thinks desired == current == initial_count and skips scaling action here
. i may be overthinking but is that a concern?
| case computeprovider.LaunchStrategyWorkerSet: | ||
| // Scale the set up so a worker comes up and registers the task queue. | ||
| if err := provider.UpdateWorkerSetSize(ctx, req.RequestContext, config, registerTaskQueueWorkerSetSize); err != nil { | ||
| recordError(wcimetrics.ErrorTypeComputeProviderFailed) | ||
| return temporal.NewApplicationErrorWithCause(fmt.Sprintf("%s: %s", k, err.Error()), "InvokeWorkerFailed", err) | ||
| } |
There was a problem hiding this comment.
with this approach, the scaling algorithm will misunderstand the current worker set size (and also this can break the existing worker pool if it already processes things).
4 participants
What was changed
Extended
InvokeWorkersToRegisterTaskQueuesto supportcomputeprovider.LaunchStrategyWorkerSet.Why?
WCI was not scaling up for Cloud Run, resulting in the task queue for the worker deployment version not being created.
Checklist
COM-231