Allow activity/workflow/nexus only workers to be registered on the same client/NS/TQ #1058
Conversation
…workers, with tests
| }); | ||
|
|
||
| // Determine worker capabilities based on configuration | ||
| let capabilities = temporalio_client::WorkerCapabilities { |
There was a problem hiding this comment.
I'm not 100% confident this is correct in how we detect workflow/nexus only workers
There was a problem hiding this comment.
Hrmm, I don't think langs tell Core today whether it will ever ask for workflow or Nexus work. This may be new (required) worker options. Never before has Core cared ahead of time which poll calls were going to be made.
There was a problem hiding this comment.
I actually kinda want to switch to that model anyway. The whole implicit polling thing complicates shutdown and switching to an up-front model would make some nasty code go away (and actually save some resources too).
The only complication would be if we ever expect allowing people to dynamically start polling for a certain kind when they weren't before... but... I'm not sure that's a scenario I think matters at all.
There was a problem hiding this comment.
The only complication would be if we ever expect allowing people to dynamically start polling for a certain kind when they weren't before... but... I'm not sure that's a scenario I think matters at all.
That could potentially be something we allow users to configure from server commands?
But would it be that be that complicated? I guess that would mean more code is kept in callbacks and more mutex/atomic variables to track state for worker commands and shutdown and such
There was a problem hiding this comment.
Yeah, good point. We can but I'm honestly not sure if we should. More knobs and complexity for unclear benefit. Though, maybe, it's kind of the same mechanism as worker "pause" which people have asked for.
I'm kinda worried about pausing/resuming polling (mostly for workflows) because shutdown is already really hard to get right and this is like extra-special shutdown.
But, regardless, I don't want you to deal with any of this right now. IMO it makes sense to add info about whether or not the worker polls for the 3 task kinds. For now, I'm perfectly fine with requiring lang to say up-front if the worker should start life polling for which task types. It can easily do that by just checking what is registered, like it does now to decide if it should make poll calls at all. If it calls poll on something it didn't say should be turned on, just immediately return PollError::Shutdown
All the other stuff we can address later
3 participants
What was changed
Add a
WorkerCapabilitiesfield to allow activity/workflow/nexus only workers to be registered on the same client/NS/TQWhy?
Even though there isn't really a valid/useful scenario where this should be used, we want to support this to minimize the breakage for users, since previously this scenario did not return an error. This is likely primarily used in tests or maybe code organization.
Checklist
Closes
How was this tested:
Added new tests
Note
Allow registering workers with non-overlapping capabilities (workflow/activity/nexus) on the same client/namespace/task queue and reserve WFT slots only from workflow-capable workers.
crates/client/src/worker_registry):WorkerCapabilitiesand require non-empty capabilities; expose viaClientWorker::worker_capabilities.namespace/task_queue/build_idif capabilities do not overlap; update error messaging.RegisteredWorkerInfo { worker_id, build_id, capabilities }instead of(Uuid, Option<String>).crates/sdk-core/src/worker/mod.rs):WorkerConfig(workflow/activities/nexus enabled) and pass throughClientWorkerRegistrator.ClientWorkerRegistratorimplementsworker_capabilities().WorkerCapabilitiesincrates/client/src/lib.rs.Written by Cursor Bugbot for commit 2b33733. This will update automatically on new commits. Configure here.