Skip to content

Initial group ID support for pollers#2422

Draft
yuandrew wants to merge 6 commits into
feature-mcnfrom
poller-group-id
Draft

Initial group ID support for pollers#2422
yuandrew wants to merge 6 commits into
feature-mcnfrom
poller-group-id

Conversation

@yuandrew

Copy link
Copy Markdown
Contributor

What was changed

  • Added poller group tracking for autoscaling pollers.
  • Plumbed poller_group_id into Workflow, Activity, and Nexus poll requests.
  • Updated pollers to consume poller_group_infos from poll responses and update local group state.
  • Initial poller group state is taken from DescribeNamespaceResponse.
  • Raised autoscaling effective minimums based on known poller groups:
    • Activity/Nexus: at least one poll per group.
    • Workflow: at least one normal poll per group.
    • Workflow with sticky enabled: at least one normal and one sticky poll per group.
  • Added workflow-specific group selection:
    • required MCN coverage is satisfied first.
    • floating workflow polls choose a group by server-provided weight.
    • sticky vs normal is then chosen from that group’s sticky backlog state.
  • Forwarded response poller_group_id for follow-up APIs that require group routing:
    • query completion
    • Nexus completion/failure

Why?

  • MCN task queues need SDK pollers to target server-provided poller groups instead of polling only the logical task queue.
  • Workflow sticky polling needs per-group coverage so workers do not get stuck polling only normal or only sticky queues for a cell.
  • Query and Nexus response APIs need the server-returned group ID so completions route back to the right server location.

Checklist

  1. Closes

  2. How was this tested:

  • Added unit coverage for poller group reservation and workflow sticky/normal selection.
  • Added worker tests for seeding poller groups from DescribeNamespaceResponse.
  • Added response-routing tests for query and Nexus poller group IDs.
  • Ran:
    • go test ./internal -run 'TestPollerGroupTrackerReserveWorkflowPoll|TestPollerGroupTrackerUpdateStickyBacklog'
    • go test ./internal -run 'TestInternalWorkerTestSuite/TestDescribeNamespacePollerGroupsSeed'
  1. Any docs updates needed?

@yuandrew yuandrew force-pushed the poller-group-id branch 2 times, most recently from 82b7507 to 1600a1b Compare July 7, 2026 17:23
tconley1428 and others added 4 commits July 9, 2026 09:33
* Automatic request header support

* Populate resource_id fields for request header routing

Implement logic to populate resource_id fields in request messages to support
multi-cell request routing based on resource IDs.

Changes:
- Workflow task requests: Populate with workflow ID from task context
- Activity task requests: Use workflow ID for workflow activities, activity ID for standalone activities
- Activity heartbeat requests: Extract resource ID from activity context
- Batch operation requests: Use workflow ID from start operation
- Worker heartbeat requests: Use worker grouping key

Added helper functions:
- getActivityResourceId(workflowId, activityId): Determines appropriate resource ID
- getActivityResourceIdFromCtx(ctx): Extracts resource ID from activity context

All resource_id fields are now properly populated to enable correct header
extraction and routing via the temporal-resource-id header.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Add resource_id population for Nexus task requests

Implement resource_id field population for Nexus task completion and failure requests.

Changes:
- Modified fillInCompletion() to accept resourceId parameter and populate RespondNexusTaskCompletedRequest.resource_id
- Modified fillInFailure() to accept resourceId parameter and populate RespondNexusTaskFailedRequest.resource_id
- Updated all calls to pass task.ResourceId from PollNexusTaskQueueResponse

The resource_id field is now populated with the value from PollNexusTaskQueueResponse.resource_id
as specified in the proto comments, enabling proper request routing via temporal-resource-id headers.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Add comprehensive activity task resource ID tests

This commit adds tests for all 8 activity task resource ID fields to ensure proper
resource_id population for multi-cell routing.

Key additions:
- Comprehensive test suite in resource_id_impl_test.go covering:
  * All 3 workflow task requests (completed, failed, query)
  * All 8 activity task requests using conversion function validation
  * Both workflow and standalone activity scenarios
- Uses convertActivityResultToRespondRequest and convertActivityResultToRespondRequestByID
  as validation points to test resource_id field population
- Tests cover RespondActivityTaskCompleted/Failed/Canceled and ByID variants
- Updated RESOURCE_ID_FIELDS.md with detailed testing status (10/15 tested, 66.7% coverage)

All tests pass and validate that:
- Workflow ID is used when present
- Activity ID is used for standalone activities (empty workflow ID)
- Conversion functions properly populate resource_id fields via getActivityResourceId helper

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Fix partial validation of RecordActivityTaskHeartbeatByIdRequest

Add comprehensive test for RecordActivityTaskHeartbeatByIdRequest resource_id field
to move from partial to full test coverage.

Changes:
- Added testActivityTaskHeartbeatByIdResourceID function with proper mocking
- Tests both workflow execution and standalone activity scenarios
- Validates resource_id field is set to workflow ID when present, activity ID when standalone
- Uses RecordActivityHeartbeatByID client method with proper service mocking
- Updated RESOURCE_ID_FIELDS.md to reflect full test coverage (11/15 tested, 73.3%)

This completes testing for all 8 activity task resource ID fields, bringing
activity task test coverage to 100% (8/8 tested).

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Remove resource_id implementations for messages without proto fields

Remove resource_id field implementations and tests for messages that no longer have
resource_id fields in the proto definitions:

- RespondQueryTaskCompletedRequest: Removed ResourceId assignment and test
- RespondNexusTaskCompletedRequest: Removed ResourceId field and unused resourceId parameter
- RespondNexusTaskFailedRequest: Removed ResourceId field and unused resourceId parameter

Also add comprehensive test for ExecuteMultiOperationRequest resource_id field,
bringing test coverage to 91.7% (11/12 implemented fields tested).

Changes:
- Remove ResourceId assignments in task handlers and pollers
- Remove unused resourceId parameters from fillInCompletion() and fillInFailure()
- Add testExecuteMultiOperationResourceID() with UpdateWithStartWorkflow validation
- Update RESOURCE_ID_FIELDS.md to reflect current proto state and test coverage

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Complete systematic verification of all resource ID field tests

Verified that all 12 resource ID fields have proper test coverage by:
- Temporarily removing ResourceId assignments
- Confirming tests fail with empty ResourceId
- Restoring ResourceId assignments
- Verifying tests pass with ResourceId restored

This ensures all resource ID tests will catch regressions where fields
are not populated correctly during request routing.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Rename test file and remove working state documentation

- Rename resource_id_impl_test.go to resource_id_test.go for conventional naming
- Remove RESOURCE_ID_FIELDS.md working state file

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Update resource ids with prefixes

* Standardize resource_id prefix responsibility in SDK

- All resource_id fields now carry fully-prefixed values from the SDK
  (e.g., "workflow:id", "worker:key", "activity:id")
- Consolidate getActivityResourceIdFromCtx to delegate to
  getActivityResourceId instead of duplicating prefix logic
- Return empty string from getActivityResourceId when both IDs are empty
- Update tests to expect prefixed values

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Update go.temporal.io/api to v1.62.6, remove local replace directive

Upgrade to published api-go v1.62.6 which includes the request header
proxy generator and standardized resource_id annotations.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Remove unused test helper and add empty resource ID test

- Remove unused createTestQueryTask helper and querypb import
- Add TestGetActivityResourceId_BothEmpty for the edge case where
  both workflowId and activityId are empty

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Address review feedback: fix CompleteActivity resource ID and nil-safety

- Pass opts.WorkflowID to convertActivityResultToRespondRequest in
  CompleteActivityWithOptions so async completions include the
  temporal-resource-id header for proxy routing. ActivityID is not
  available on this code path (task-token based identification).
- Use nil-safe getter GetWorkflowId() instead of direct field access
  on task.WorkflowExecution in completeWorkflow, consistent with all
  other usages in this file.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Address resource ID review feedback

---------

Co-authored-by: Chad Retz <chad@temporal.io>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants