Repository navigation
feat: add external storage concurrency limiter APIs - #615
Open
jmaeagle99 wants to merge 1 commit into
Open
jmaeagle99 wants to merge 1 commit into
jmaeagle99 wants to merge 1 commit into
Conversation
GregoryTravis
approved these changes
Oct 13, 2026
chris-olszewski
approved these changes
Oct 14, 2026
chris-olszewski
left a comment
chris-olszewski
left a comment
Member
There was a problem hiding this comment.
Just small questions around some doc comment language
Comment on lines
+20
to
+21
| # Take one permit per request rather than one around a whole batch. Wrapping an entire fan out in a single | ||
| # permit satisfies this signature while defeating the limit it exists to impose. Taking a permit while already |
Member
There was a problem hiding this comment.
If the second sentence explaining why to take a permit per request is there for consistency with other SDKs, that seems fine, but otherwise I think the first sentence is all that is necessary.
| # waiting when the SDK abandons the operation stops waiting. | ||
| # | ||
| # @param item [Api::Common::V1::Payload, StorageDriverClaim] What this one request covers: a payload when | ||
| # storing, a claim when retrieving. A limiter may price a permit by it; one that does not may ignore it. |
Member
There was a problem hiding this comment.
Might be a me thing, but "price a permit" is not language I've seen before.
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
3 participants
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
Adds the concurrency contract for external storage drivers:
StorageDriverLimiter, withpermit(item) { ... }holding a permit for the block.ExternalStorageConcurrency, carrying the two budgets:max_driver_operations(64, sharedacross the configuration) and
max_operations_per_message(8, per activation/task/request).StorageDriverStoreContextandStorageDriverRetrieveContext, andExternalStoragetakes aconcurrency:.Why?
A driver is handed a whole batch and decides for itself how many requests to make of it, so the SDK can only bound that fan out cooperatively by handing the driver something to take a permit from. The spec puts the limiter on the store and retrieve contexts and the budgets on the storage configuration.
Ruby's block form is the counterpart of Python's
async with limiter.permit(item):— both hold the permit for a lexical scope and release it on exit, including when the body raises. Field names and defaults match Python'sExternalStorageConcurrencyexactly. No cancellation parameter is needed because the Ruby contexts already carry aCancellation.Checklist
How was this tested: New unit tests cover the defaults (64 / 8), rejection of non-positive and non-Integer budgets, and that
permitruns the block and returns its value.Any docs updates needed? Not yet. Everything here is
@!visibility privateand experimental.