feat!(extstore): add new concurrency controls for external storage. - #3114
Open
cconstable wants to merge 2 commits into
Open
cconstable wants to merge 2 commits into
cconstable wants to merge 2 commits into
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was changed
ExternalStoragenow has aConcurrencyfield with two limits:MaxDriverOperationscaps requests in flight across every driver on that ExternalStorage instance.MaxOperationsPerMessagecaps requests for a single message (workflow task, etc). Each message gets its own budget.Both are limits are cooperative. Drivers receive a Limiter on the store and retrieve contexts and must wrap each request in Permit. Requests made outside a permit are not counted, and a warning is logged when a driver completes without taking one.
Removed WorkerOptions.MaxConcurrentWorkflowTaskExternalStorageVisits. The payload visitor now walks all payload sites in a message concurrently, since the two limits above do the bounding. The bundled S3 and GCS drivers take permits, and the GCS driver's hardcoded limit of 10 is gone.
Why?
The previous control bounded payload visits (not storage operations or driver calls) which was a bit unintuitive and left driver fanout unbounded (e.g. setting the previous limit to 3 did not limit concurrent external storage requests to 3). It also only applied to workflow tasks, leaving client, activity, and Nexus paths unbounded.
Checklist
Added new tests.