refactor: simplify the init-concurrency limiter to a single shared budget - #785
Open
kinyoklion wants to merge 1 commit into
Open
refactor: simplify the init-concurrency limiter to a single shared budget#785kinyoklion wants to merge 1 commit into
kinyoklion wants to merge 1 commit into
Conversation
…dget The limiter primitive added in #780 carried an optional per-environment gate and a matching INIT_PER_ENV_MAX_PERCENT config option. Drop per-environment fairness: the limiter now bounds concurrent admissions with one shared budget (a fixed number of held slots plus a bounded FIFO queue of waiters), and Acquire no longer takes an environment key. Also revise the INIT_SEND_TIMEOUT documentation and default to describe an absolute cap on how long one gated delivery may hold a slot. No runtime effect on its own: nothing acquires from the limiter yet.
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.
Refines the init-concurrency limiter primitive and configuration that #780 landed on the feature branch, in preparation for wiring it into the poll and stream endpoints.
What changes
PerEnvMax) and a matchingINIT_PER_ENV_MAX_PERCENTconfig option. Both are removed. The limiter now bounds concurrent admissions with a single shared budget: a fixed number of held slots plus a bounded FIFO queue of waiters.Acquireno longer takes an environment key.INIT_SEND_TIMEOUTdocumentation and default now describe it as an absolute cap on how long one gated delivery may hold a slot.Why one shared budget
The budget we need to protect is Relay's overall memory and egress during a reconnect herd; that is a global resource, not a per-environment one. A per-environment gate rejects an environment that is under the global budget while slots sit free, and it adds bookkeeping (a
sync.Mapof gates) for a fairness property we do not need here. A single shared budget is simpler and matches the resource it guards.No behavior change
Nothing acquires from the limiter yet, so this is inert at runtime. It builds, vets, and passes
-raceon./config/and./internal/concurrency/.First of a set of PRs splitting the larger wiring change (#782) into independently reviewable pieces. This one stands alone.
Note
Low Risk
Refactor of unused limiter/config surface with no production call sites; main operational note is breaking removal of
INIT_PER_ENV_MAX_PERCENTfor anyone who set it.Overview
Simplifies the init-concurrency limiter ahead of poll/stream wiring by dropping per-environment fairness in favor of one global admission budget.
Removed:
INIT_PER_ENV_MAX_PERCENT/PerEnvMaxPercent, limiterPerEnvMax, per-envsync.Mapgates, and theenvKeyargument onAcquire. Admission is now only MaxConcurrent held slots plus a bounded FIFO queue (MaxQueued).INIT_SEND_TIMEOUTdocs now describe an absolute cap on how long a gated delivery may hold a slot (default 2m), with a 64 KB/s throughput floor expected to reclaim slow/stalling clients sooner.Tests drop per-env isolation coverage and call
Acquire(ctx)without an environment key. No runtime behavior change yet—nothing acquires from this limiter in production paths.Reviewed by Cursor Bugbot for commit 8a9ec78. Bugbot is set up for automated code reviews on this repo. Configure here.