Skip to content

Conversation

@Shivs11
Copy link
Member

@Shivs11 Shivs11 commented Dec 10, 2025

What changed?

  • WISOTT
  • Also added a cache per history host so that we don't overburden matching with these calls.
  • Also added a whole new unit test testing the function ValidateVersioningOverride

Why?

  • Versioning correctness.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

Potential risks

  • I don't think this is risky

Note

Adds CheckTaskQueueVersionMembership RPC and uses it (with per-host TTL cache) to validate VersioningOverride only when the version exists; wires through clients/handlers and updates tests.

  • API/Protos:
    • Add CheckTaskQueueVersionMembershipRequest/Response and service RPC CheckTaskQueueVersionMembership in matchingservice.
  • Matching Service:
    • Implement RPC to read task queue user data and determine if a version is present.
    • Expose in gRPC server/client and gomock; add quota entry.
  • Clients:
    • Add client, metric, and retryable wrappers; route via partitioned client.
  • History:
    • Validate VersioningOverride by calling the new RPC; add per-host cache (TTL via history.versionMembershipCacheTTL).
    • Move/centralize validation into history paths (StartWorkflowExecution, SignalWithStart, Reset, MultiOperation, UpdateWorkflowExecutionOptions).
    • Remove prior frontend/batcher-side validations; include non-retryable batch error hint.
  • Config:
    • Add dynamic config VersionMembershipCacheTTL and initialize cache in history engine.
  • Testing/Utils:
    • Add helpers (WithDeploymentSeries, WithBuildID).
    • Add unit tests for ValidateVersioningOverride and functional tests covering pinned overrides, batch updates, reset, and the new RPC.

Written by Cursor Bugbot for commit 5c31e16. This will update automatically on new commits. Configure here.

Comment on lines +4796 to +4797
// TODO (Shivam): Verify if this is the correct way to handle these errors.
input.NonRetryableErrors = append(input.NonRetryableErrors, "Pinned version is not present in the task queue")
Copy link
Member Author

@Shivs11 Shivs11 Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would love to get some clarity over here - theoretically, this works as I don't want the internal batch workflow's activity to retry on these errors. But should something like this be done in this manner is a different question that I am trying to answer.

Would love to get someone's opinion here - have also mentioned this in our slack channel

}

func (s *WorkflowAliasSearchAttributeTestSuite) TestWorkflowAliasSearchAttribute_CustomSearchAttributeOverride() {
func (s *WorkflowAliasSearchAttributeTestSuite) TestWorkflowAliasSearchAttribute_CustomSAOverride() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

had to make the test name shorter here otherwise there were registration errors in our worker-versioning entity workflows since we have some length checks

Comment on lines -497 to -501
// Validate versioning override, if present.
if err := worker_versioning.ValidateVersioningOverride(request.GetVersioningOverride()); err != nil {
return nil, err
}

Copy link
Member Author

@Shivs11 Shivs11 Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disclaimer for reviewer: all validation checks that you see removed from the frontend, in this PR, are simply moved from frontend to history.

Coincidentally, I had worked on this which had initially moved them from history to frontend. Little did I know that I was wrong and that the authors of these changes had some incredible foresight (or maybe they did not and were just lucky)

@Shivs11 Shivs11 marked this pull request as ready for review December 11, 2025 03:22
@Shivs11 Shivs11 requested review from a team as code owners December 11, 2025 03:22
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