Add TaskListPartitionConfig message to proto #6358
Conversation
Shaddoll
requested review from
neil-xie,
davidporter-id-au,
Groxx,
shijiesheng,
jakobht,
3vilhamster,
sankari165,
dkrotx,
taylanisikdemir and
demirkayaender
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
| @@ -290,7 +290,7 @@ func (e *mutableStateBuilder) tryDispatchActivityTask( | |||
| metrics.WorkflowTypeTag(e.GetWorkflowType().Name), | |||
| metrics.TaskListTag(ai.TaskList)) | |||
| taggedScope.IncCounter(metrics.DecisionTypeScheduleActivityDispatchCounter) | |||
| err := e.shard.GetService().GetMatchingClient().AddActivityTask(ctx, &types.AddActivityTaskRequest{ | |||
| _, err := e.shard.GetService().GetMatchingClient().AddActivityTask(ctx, &types.AddActivityTaskRequest{ | |||
There was a problem hiding this comment.
At which layer we will be caching these results and make use of it? If it's going to be done by a matching client wrapper then we probably don't need to change matching.Client interface and the callers wouldn't change. I may be missing something (later review)
There was a problem hiding this comment.
it's done in matching.Client. Since I've made the change, let's keep it. It's always better to have an empty response than void
There was a problem hiding this comment.
Discussed offline. We may actually need this response to record the identity of Started tasks for a feature that reduces amount of transfer tasks.
Detailed Description
Introduce a new type TaskListPartitionConfig to protobuf and add the new type as a new field to the following proto messages:
Impact Analysis
Testing Plan
Rollout Plan
Thrift is not updated because the AddXXXTask methods return void. Besides, we're going to deprecate thrift and tchannel. So the decision is to add less feature support for thrift.