Expose Poller Automation #1691
Expose Poller Automation #1691
Conversation
| 'Second workflow poller count should be 1 or 2' | ||
| ); | ||
|
|
||
| const workflowPromises = Array(20) |
There was a problem hiding this comment.
There's no test assertion pass this point. I suppose you only want to confirm it doesn't throw, but should at least do t.pass() or some other trivial check.
There was a problem hiding this comment.
I always forget that's a thing. Seems so nonsense to me that you'd have to call pass explicitly.
There was a problem hiding this comment.
That's technically not required in this specific case, as you already have some assertions earlier in the test, so yes, t.pass() will basically just be a noop, but it still clearly communicate to future readers that the absence of specific assertions in the second half of that test is intentional.
| }); | ||
|
|
||
| await Promise.all(workflowPromises); | ||
| worker.shutdown(); |
There was a problem hiding this comment.
I'm pretty sure localEnv.teardown() below will hang the process if worker.shutdown() isn't called first (e.g. because of a thrown error above). Though that should be fixed by the refactor, so feel free to just ignore if that's not causing any harm to you.
| * A poller behavior that will automatically scale the number of pollers based on feedback | ||
| * from the server. A slot must be available before beginning polling. | ||
| */ | ||
| export interface PollerBehaviorAutoscaling { |
There was a problem hiding this comment.
Should this behavior marked as experimental?
| maximum: behavior.maximum ?? 100, | ||
| }; | ||
|
|
||
| const wftPollerBehavior = createPollerBehavior(maxWFTPolls, workflowTaskPollerBehavior); |
There was a problem hiding this comment.
Is this per WFT poller types? Is there a risk that we might starve either regular or sticky task queue pollers (i.e. if min/initial/max set to 1)?
There was a problem hiding this comment.
Also, I assume invalid configuration will be reported by Core?
There was a problem hiding this comment.
This applies to each independently, starvation won't occur. Yes, invalid config is reported by Core.
|
Are we considering making autoscaling 1/5/100 be the default at some point? I'd expect that to give much better performance than our current default values and heuristics. |
Yes, it should become the default eventually |
Sushisource
force-pushed
the
expose-poller-automation
branch
2 times, most recently
from
c4fbec3 to
0f27727
Compare
Sushisource
force-pushed
the
worker-deployment-versioning
branch
from
e7f4dd6 to
218743e
Compare
Sushisource
force-pushed
the
expose-poller-automation
branch
from
0f27727 to
3ed9b7c
Compare
Sushisource
force-pushed
the
worker-deployment-versioning
branch
from
218743e to
8169797
Compare
|
No longer needed |
Needs #1679 to merge first
What was changed
Expose options to opt-in to new poller autoscaling behavior
Why?
Poller autoscaling is cool!
Checklist
Closes
How was this tested:
Added simple test. More meaningful tests are in Core, we just need to make sure option is passed properly here.
Any docs updates needed?