Matching logic for pinned workflows #6853
Merged
+459
−150
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…g-behaviors # Conflicts: # api/deployment/v1/message.go-helpers.pb.go # api/deployment/v1/message.pb.go # common/worker_versioning/worker_versioning.go # proto/internal/temporal/server/api/deployment/v1/message.proto
…g-behaviors # Conflicts: # api/persistence/v1/executions.pb.go # service/matching/task_queue_partition_manager.go
ShahabT
changed the base branch from
versioning-3
to
shahab/v3-internal-protos
…shahab/transfer-versioning-info # Conflicts: # service/history/workflow/mutable_state_impl.go
ShahabT
changed the base branch from
shahab/v3-internal-protos
to
shahab/transfer-versioning-info
alexshtin
reviewed
Nov 23, 2024
Comment on lines
+41
to
+45
| nonRootPartitionPrefix = "/_sys/" | ||
| partitionDelimiter = "/" | ||
| versionSetDelimiter = ":" | ||
| buildIdDelimiter = "#" | ||
| deploymentNameDelimiter = "|" |
There was a problem hiding this comment.
This is really cool! Let's aim to draw something meaningful here. Like a Christmas tree.
alexshtin
reviewed
Nov 23, 2024
Comment on lines
+45
to
+75
| func (s *VersioningIntegSuite) startWorkflow() *commonpb.WorkflowExecution { | ||
| id := s.randomizeName("my-wf") | ||
| wt := "MyWfType" | ||
| tqName := "my-tq" | ||
| tq := &taskqueuepb.TaskQueue{Name: tqName, Kind: enumspb.TASK_QUEUE_KIND_NORMAL} | ||
| identity := "worker1" | ||
|
|
||
| request := &workflowservice.StartWorkflowExecutionRequest{ | ||
| RequestId: uuid.New(), | ||
| Namespace: s.Namespace(), | ||
| WorkflowId: id, | ||
| WorkflowType: &commonpb.WorkflowType{Name: wt}, | ||
| TaskQueue: tq, | ||
| Input: nil, | ||
| WorkflowRunTimeout: durationpb.New(100 * time.Second), | ||
| WorkflowTaskTimeout: durationpb.New(1 * time.Second), | ||
| Identity: identity, | ||
| } | ||
|
|
||
| we, err0 := s.FrontendClient().StartWorkflowExecution(testcore.NewContext(), request) | ||
| s.NoError(err0) | ||
| return &commonpb.WorkflowExecution{ | ||
| WorkflowId: id, | ||
| RunId: we.GetRunId(), | ||
| } | ||
| } | ||
|
|
||
| // Adds the test name and a random string as postfix to the given name | ||
| func (s *VersioningIntegSuite) randomizeName(name string) string { | ||
| return testcore.RandomizeStr(name + "_" + s.T().Name()) | ||
| } |
There was a problem hiding this comment.
you mean randomizeName? why? :D
There was a problem hiding this comment.
This is an incomplete test because I realize after starting it that I need TaskPoller to finish the test. So I'm waiting to merge the feature branch to main so then I complete the tests.
There was a problem hiding this comment.
I think he means we should use testvars instead?
There was a problem hiding this comment.
I see, sure will use testvars once I get to writing tests.
…er-versioning-info # Conflicts: # api/deployment/v1/message.pb.go # api/historyservice/v1/request_response.pb.go # api/persistence/v1/tasks.pb.go # api/taskqueue/v1/message.pb.go # proto/internal/temporal/server/api/deployment/v1/message.proto # proto/internal/temporal/server/api/historyservice/v1/request_response.proto
…haviors # Conflicts: # common/worker_versioning/worker_versioning.go
…er-versioning-info
Base automatically changed from
shahab/transfer-versioning-info
to
versioning-3
November 25, 2024 19:57
…g-behaviors # Conflicts: # common/worker_versioning/worker_versioning.go # service/history/workflow/util.go
dnr
reviewed
Nov 26, 2024
Comment on lines
+45
to
+75
| func (s *VersioningIntegSuite) startWorkflow() *commonpb.WorkflowExecution { | ||
| id := s.randomizeName("my-wf") | ||
| wt := "MyWfType" | ||
| tqName := "my-tq" | ||
| tq := &taskqueuepb.TaskQueue{Name: tqName, Kind: enumspb.TASK_QUEUE_KIND_NORMAL} | ||
| identity := "worker1" | ||
|
|
||
| request := &workflowservice.StartWorkflowExecutionRequest{ | ||
| RequestId: uuid.New(), | ||
| Namespace: s.Namespace(), | ||
| WorkflowId: id, | ||
| WorkflowType: &commonpb.WorkflowType{Name: wt}, | ||
| TaskQueue: tq, | ||
| Input: nil, | ||
| WorkflowRunTimeout: durationpb.New(100 * time.Second), | ||
| WorkflowTaskTimeout: durationpb.New(1 * time.Second), | ||
| Identity: identity, | ||
| } | ||
|
|
||
| we, err0 := s.FrontendClient().StartWorkflowExecution(testcore.NewContext(), request) | ||
| s.NoError(err0) | ||
| return &commonpb.WorkflowExecution{ | ||
| WorkflowId: id, | ||
| RunId: we.GetRunId(), | ||
| } | ||
| } | ||
|
|
||
| // Adds the test name and a random string as postfix to the given name | ||
| func (s *VersioningIntegSuite) randomizeName(name string) string { | ||
| return testcore.RandomizeStr(name + "_" + s.T().Name()) | ||
| } |
There was a problem hiding this comment.
I think he means we should use testvars instead?
dnr
approved these changes
Nov 26, 2024
dnr
pushed a commit
that referenced
this pull request
Nov 26, 2024
## What changed? <!-- Describe what has changed in this PR --> Handling tasks of pinned workflows. ## Why? <!-- Tell your future self why have you made these changes --> Part of versioning 3. ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> Tests will come in a followup PR. ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changed?
Handling tasks of pinned workflows.
Why?
Part of versioning 3.
How did you test it?
Tests will come in a followup PR.
Potential risks
Documentation
Is hotfix candidate?