Integration test for workflow ID based rate limiting task processing #5933
Conversation
sankari165
requested review from
Shaddoll,
neil-xie,
davidporter-id-au,
Groxx,
shijiesheng,
agautam478,
jakobht,
3vilhamster,
dkrotx,
taylanisikdemir and
demirkayaender
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted filessee 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
| history := historyResponse.History | ||
| firstEvent := history.Events[0] | ||
| lastEvent := history.Events[len(history.Events)-1] | ||
| // First 7 event ids --> 0 (Workflow start), 1-3 ( Decision scheduled,started,completed) , 4-6 (Activity scheduled,started,completed) post which the tasks will be rate limited since RPS is 2 |
There was a problem hiding this comment.
it's not super clear to me how this test validates workflow id rate limits. how do you ensure these specific events are spread with 2 rps?
There was a problem hiding this comment.
These events correspond to two tasks - one decision task and one activity task. The ratelimiter is at the task processing level and after processing 2 tasks, the rate limiter starts to return an error
There was a problem hiding this comment.
I guess there's no externally visible error being returned, just a delay?
There was a problem hiding this comment.
yes there is no externally visible error since the rate limit error is being retried and delays the task processing
| s.Logger.Info("Waiting for workflow to complete", tag.WorkflowRunID(we.RunID)) | ||
|
|
||
| s.False(workflowComplete) | ||
| _, err = poller.PollAndProcessDecisionTask(false, false) |
There was a problem hiding this comment.
I'm not super familiar with these engine tests. Does this block here waiting for a reponse (for N seconds or whatever) until there's a task?
There was a problem hiding this comment.
It has a fixed 5 attempts and then says no tasks found
Pull Request Test Coverage Report for Build 018f1a68-5336-41ff-8437-f9fa1d1611d9Details
💛 - Coveralls |
What changed?
New integration test for verifying rate limiting of task processing
Why?
testing
How did you test it?
Ran it locally
Potential risks
No risks with testing
Release notes
Documentation Changes