Increase number of forward tokens for isolation tasklists #5310
Conversation
Pull Request Test Coverage Report for Build 01887cfb-f41d-4b04-991b-c636745b5a60
💛 - Coveralls |
| @@ -91,19 +93,21 @@ func newForwarder( | |||
| taskListID *taskListID, | |||
| kind types.TaskListKind, | |||
| client matching.Client, | |||
| isolationGroups []string, | |||
There was a problem hiding this comment.
Is this intended to be that private-package type? Would you mind renaming this parameter to indicate it's purpose, as the term 'isolation-groups' can mean multiple things - the groups associated with a service which are available, all the isolation-groups, the poller-available isolation-groups etc.
(I am guessing this is all?)
There was a problem hiding this comment.
it's allisolationgroups if isolation is enabled, it's nil if not
| ) *Forwarder { | ||
| rpsFunc := func() float64 { return float64(cfg.ForwarderMaxRatePerSecond()) } | ||
| fwdr := &Forwarder{ | ||
| cfg: cfg, | ||
| client: client, | ||
| taskListID: taskListID, | ||
| taskListKind: kind, | ||
| outstandingTasksLimit: int32(cfg.ForwarderMaxOutstandingTasks()), | ||
| outstandingPollsLimit: int32(cfg.ForwarderMaxOutstandingPolls()), | ||
| outstandingTasksLimit: int32(cfg.ForwarderMaxOutstandingTasks() * (len(isolationGroups) + 1)), |
There was a problem hiding this comment.
Mind adding a comment here as to the intuition of this?
From the context I'm guessing that you're saying that the limit should be larger for tasklists when isolation-groups are enabled so as to prevent thrash for partitioned tasklists, but it's not immediately obvious looking at the code and it's surrounds
There was a problem hiding this comment.
there is an intuition in line 264
What changed?
Increase number of forward tokens for isolation tasklists
Why?
Fix the latency degradation for scalable tasklists with isolation
How did you test it?
staging2 test
Potential risks
Release notes
Documentation Changes