Auto-enroll pollers into autoscaling on PollerAutoscalingAutoEnroll (Java SDK)#2953
Conversation
When a namespace advertises the PollerAutoscalingAutoEnroll capability, automatically switch a poller type to poller autoscaling if the user left it at its default (set neither MaxConcurrent<Type>TaskPollers nor a <Type>TaskPollerBehavior). Explicitly configured pollers are untouched. Auto-enroll implies full autoscaling support, so it also enables serverSupportsAutoscaling (scale-down). Applies to workflow, activity, and nexus pollers. The decision is made at worker start(), after namespace capabilities are loaded, by rebuilding each dormant worker's PollerOptions before its poller is created. Per-poller-type eligibility is captured at Worker construction from the raw (pre-defaulting) WorkerOptions and threaded through PollerOptions. Also bumps the temporal/api proto submodule to pick up the poller_autoscaling_auto_enroll namespace capability (api #803).
veeral-patel
changed the title
| pollerAutoscalingAutoEnroll.set(true); | ||
| } | ||
| // Auto-enroll implies full poller autoscaling support, including scaling down, so it also | ||
| // enables pollerAutoscaling (which drives serverSupportsAutoscaling in PollScaleReportHandle). |
There was a problem hiding this comment.
not entirely sure the purpose for this comment, why is "scaling down" relevant here?
There was a problem hiding this comment.
Removed the comment, thank you
| // at start() when the namespace advertises the PollerAutoscalingAutoEnroll capability. This | ||
| // must | ||
| // be read from the raw (pre-defaulting) options, since validateAndBuildWithDefaults() fills in | ||
| // a |
There was a problem hiding this comment.
spacing is weird on this comment
| boolean workflowTaskAutoEnrollEligible = | ||
| options == null | ||
| || (options.getWorkflowTaskPollersBehavior() == null |
There was a problem hiding this comment.
I think we need to preserve a "setter was explicitly called" as private/internal metadata through build, validation, and copy-building, then transfer that into the internal PollerOptions. Only WorkerOptions.Builder knows whether the user actually called either poller setter.
There was a problem hiding this comment.
Good idea, this is done.
I added a private {workflow,activity,nexus}TaskPollersConfigured flag, set to true inside both setters for each type (setMaxConcurrentTaskPollers and setTaskPollersBehavior).
| @@ -0,0 +1,138 @@ | |||
| package io.temporal.worker; | |||
There was a problem hiding this comment.
Can we add tests showing that pollers remain eligible when default options come from WorkerOptions.getDefaultInstance(), validateAndBuildWithDefaults(), or a copy of either? And a test that explicitly sets the count to the numerical default of 5 proving its ineligible.
It would also be useful to have one startup-path test that enables the namespace capability, starts workflow/activity/Nexus workers, and verifies their effective poller behavior becomes autoscaling. I think we're missing a full E2E wiring test here.
There was a problem hiding this comment.
Can we add tests showing that pollers remain eligible when default options come from WorkerOptions.getDefaultInstance(), validateAndBuildWithDefaults(), or a copy of either? And a test that explicitly sets the count to the numerical default of 5 proving its ineligible.
Done, added the following test cases under WorkerPollerAutoEnrollEligibilityTest:
| Test case | Summary | Expected |
|---|---|---|
defaultInstanceMakesEveryPollerTypeEligible |
getDefaultInstance() carries the numeric default count (5), but no setter was called — so intent-tracking keeps it eligible |
Eligible (all types) |
validateAndBuildWithDefaultsMakesEveryPollerTypeEligible |
Same for newBuilder().validateAndBuildWithDefaults(): defaulting fills in 5 without marking the pollers configured |
Eligible (all types) |
copyOfDefaultInstanceIsEligible |
Copying a default-options object (via both build() and validateAndBuildWithDefaults()) preserves the "no setter called" state, so the copy stays eligible too |
Eligible (all types) |
explicitCountEqualToNumericDefaultIsIneligible |
User explicitly calls setMaxConcurrentWorkflowTaskPollers(5) — same value as the default, but now the setter was invoked, so it must count as configured |
Ineligible (workflow); others eligible |
It would also be useful to have one startup-path test that enables the namespace capability, starts workflow/activity/Nexus workers, and verifies their effective poller behavior becomes autoscaling. I think we're missing a full E2E wiring test here.
Done, added this test: WorkerPollerAutoEnrollStartupTest.autoEnrollAtStartupSwitchesPollersToAutoscaling.
It starts a Worker (with a NamespaceCapabilities advertising the capability) that has workflow, activity, and nexus implementations registered, then asserts all three pollers' effective behavior is PollerBehaviorAutoscaling after start(). This covers the full path: WorkerOptions → eligibility → start-time enrollment → AsyncPoller.
- Determine auto-enroll eligibility from whether the user actually called a poller setter (MaxConcurrent<Type>TaskPollers or <Type>TaskPollerBehavior), tracked on WorkerOptions.Builder and carried through build/validate/copy, instead of inferring from the resolved option values. This fixes the case where options from getDefaultInstance()/validateAndBuildWithDefaults() carry the numeric default (5) and would wrongly look explicitly configured. - Worker reads the new WorkerOptions eligibility getters instead of the raw pre-defaulting options. - Reword the NamespaceCapabilities comment to explain why auto-enroll also enables pollerAutoscaling (serverSupportsAutoscaling in PollScaleReportHandle), and fix the mangled comment in Worker. - Expand tests: eligibility from getDefaultInstance()/validateAndBuildWithDefaults() and copies stays eligible; an explicit count equal to the numeric default is ineligible. Add a full start-path E2E test asserting workflow/activity/nexus pollers become autoscaling when the namespace advertises the capability.
The auto-enroll capability no longer forces NamespaceCapabilities.pollerAutoscaling on. The server advertises pollerAutoscaling independently (and unconditionally), so the pre-existing getPollerAutoscaling() read already enables scale-down for auto-enrolled pollers; the extra coupling was redundant and conflated the scale-down flag with the enrollment decision. setFromCapabilities reverts to reading each capability separately. The auto-enroll field/getter stay, since the enrollment decision still needs them and is independent of isPollerAutoscaling(). Replaced the obsolete implication test with one asserting the two capabilities are independent.
|
Tested this change manually successfully. Setup: Built temporal server from my branch, 1 worker, 2 namespaces. Examined the logs in order to check. default namespace: autoscale-ns namespace: |
3 participants
Mirrors the Go SDK (temporalio/sdk-go#2442).
What
When a namespace advertises the
PollerAutoscalingAutoEnrollcapability, workers automatically use poller autoscaling for any poller type left at its default — i.e. where the user set neitherMaxConcurrent<Type>TaskPollersnor a<Type>TaskPollerBehavior. Explicitly configured pollers are left alone. Applies to workflow, activity, and nexus pollers.How
Workerrecords, per poller type, whether it was left at its default (read from the rawWorkerOptions, before defaults are applied).start()— once namespace capabilities are known — each eligible poller's behavior is swapped to autoscaling before its poller is created.NamespaceCapabilitiestreats it as enablingpollerAutoscalingtoo.Proto
Bumps the
temporal/apisubmodule to pick up thepoller_autoscaling_auto_enrollcapability (api #803).Testing
PollerAutoscalingAutoEnrollTest— the enroll decision (default + capability → autoscaling; capability off, explicit count, or explicit behavior → unchanged).WorkerPollerAutoEnrollEligibilityTest—Workermarks defaults eligible and explicit config ineligible, per type.