Activity Priorities #137
Activity Priorities #137
Conversation
| return fmt.Errorf("failed to parse %v: %w", VisibilityVerificationTimeout, err) | ||
| } | ||
| timeout := time.Duration(1*internalIterations) * internalIterTimeout | ||
|
|
There was a problem hiding this comment.
Moved all of these to the top so (a) if there's an error, it aborts immediately and (b) so it's not re-parsed on every run.
7f86266 to
def37fd
Compare
| // If set, the workflow will run the "Sleep" Activity with the given sleep patterns per priority. | ||
| SleepActivityPerPriority SleepActivity[int] `json:"sleepActivityPerPriority"` |
There was a problem hiding this comment.
I think we'll have another one for the vanilla use case (no priority/fairness), and another one for fairness later.
| func MakeSleepInput(distribution throughputstress.SleepActivity[int]) *SleepActivityInput { | ||
| prio, ok := distribution.PatternsDist.Sample() | ||
| if !ok { | ||
| return nil | ||
| } | ||
| sleep, ok := distribution.PatternDurationsDist[prio].Sample() | ||
| if !ok { | ||
| return nil | ||
| } | ||
| fmt.Println("priority", prio, "sleep", sleep) | ||
| return &SleepActivityInput{Priority: prio, SleepDuration: sleep} | ||
| } |
There was a problem hiding this comment.
New activity that can controlled via SleepActivityPerPriorityJsonFlag
There was a problem hiding this comment.
LGTM, nothing blocking (I also apply low scrutiny to code in this repo vs, say, public facing API). But may want to wait for @Sushisource and/or others who are more familiar with this repo and use it more.
loadgen/helpers.go
Outdated
| type ( | ||
| Weight int | ||
| DistType interface{ ~int | ~string | time.Duration } | ||
| Distribution[T DistType] map[T]Weight | ||
| ) |
There was a problem hiding this comment.
May need some short docs on what these things are, it's a bit confusing to me as an outsider scanning the code
| // The endpoint should be created ahead of time. | ||
| NexusEndpoint string `json:"nexusEndpoint"` | ||
| } | ||
| type ( |
There was a problem hiding this comment.
Why are we switching to this type ( approach instead of type keyword per type?
There was a problem hiding this comment.
I was under the impression it was idiomatic Go. No strong opinion myself here.
There was a problem hiding this comment.
Also no strong opinion, though I don't find it very idiomatic IMO
loadgen/helpers.go
Outdated
| type ( | ||
| Weight int | ||
| DistType interface{ ~int | ~string | time.Duration } | ||
| Distribution[T DistType] map[T]Weight | ||
| ) |
| scenarioOptions[pieces[0]] = pieces[1] | ||
| key, value := pieces[0], pieces[1] | ||
|
|
||
| 8000 | if strings.HasPrefix(value, "@") { |
There was a problem hiding this comment.
This needs to get documented somewhere... I'm not sure if it makes sense to do on every option that can take JSON or if there's some general place that would make more sense, but, we should have it mentioned somewhere.
There was a problem hiding this comment.
I've added it to the README in the context of the throughput_stress scenario now.
loadgen/helpers.go
Outdated
| "time" | ||
|
|
||
| "go.temporal.io/api/workflowservice/v1" | ||
| "go.temporal.io/sdk/client" | ||
| ) | ||
|
|
||
| type ( | ||
| Weight int | ||
| DistType interface{ ~int | ~string | time.Duration } |
There was a problem hiding this comment.
TIL about that interface syntax. Kinda cool
<!--- Note to EXTERNAL Contributors --> <!-- Thanks for opening a PR! If it is a significant code change, please **make sure there is an open issue** for this. We work best with you when we have accepted the idea first before you code. --> <!--- For ALL Contributors 👇 --> ## What was changed <!-- Describe what has changed in this PR --> Follow-up to #137. Adds more distribution types: normal, zipf and uniform. Sorry for breaking the JSON schema already after I just merged it yesterday, but I doubt anyone is using this yet. ## Why? <!-- Tell your future self why have you made these changes --> For testing fairness, we'll need the ability to test millions of distinct keys. The discrete distribution alone is insufficient for that. ## Test ``` echo '{"patterns_dist": {"type":"discrete", "weights": {"1":1, "5":9}}, "pattern_durations_dist": {"1": {"type":"discrete", "weights": {"1s": 1}}, "5": {"type":"discrete", "weights": {"1s": 1, "5s": 4}}}}' > input.json go run ./cmd run-scenario-with-worker \ --scenario throughput_stress --run-id omes1 --language go \ --option sleep-activity-per-priority-json=@input.json ```
What was changed
SleepActivityPerPriorityJsonFlagVisibilityVerificationTimeoutWhy?
Allow to test priority feature with different traffic patterns.
Example
Added a print statement, some sample output: