Repository navigation
docs: Queued admission with a token refill rate is FIFO as of v1.12.0; docs still anchor it to unreleased main - #170
Merged
Conversation
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions
Bot
requested review from
CatherineSue,
key4ng and
slin1237
as code owners
October 13, 2026 11:17
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Contributor
Author
|
Documentation maintenance: ready. Unsuccessful content rounds: 0/3. Operational attempts: 0/3. Head: The PR re-anchors the FIFO-refill note on rate-limiting.md from "main after v1.11.0" to "Changed in v1.12.0" and nothing else; both hunks serve that one concern. The pinned release checkout (HEAD dcc5cb0b = tag v1.12.0 per .git/packed-refs) contains the f3723c52 behavior: token_bucket.rs:16-27 and acquire (196-255) implement unconditional FIFO waiter handoff with only the front waiter arming a refill timer, try_acquire_sync:179 blocks barging while waiters exist, and concurrency.rs:217,228-237 bounds queued waits by queue_timeout with 503 admission_queue_timeout — so "queue order and wait bound apply in both modes" is release-accurate. The retained v1.11.0-and-earlier caveat matches the source_patch pre-image (poll loop, wait ≤ 1/N via Elapsed, new arrivals able to take refilled tokens). Placement follows docs style: release note stays in its own admonition below the table, current behavior stated first, the commit link smg#2733 removed, no code-change references added, established-behavior sentence freed of the appended caveat; ~4 changed lines on the canonical page, no new page. Related pages are consistent: metrics.md:236's unconditional FIFO sentence is now correct at v1.12.0, configuration.md:489 and reliability-controls.md:36 describe only the in-flight-exceeds-cap effect, and no other page mentions refill queue ordering. Limitation: with Read/Glob/Grep only I could not run a git ancestry proof that f3723c52 is absent from the v1.11.0 tag; the boundary is supported by the source-patch pre-image, the previously merged note anchoring the change after v1.11.0, and the commit's presence in the v1.12.0 checkout. No feedback threads exist on this PR. Human review threads and CODEOWNER approval remain under repository policy. |
2 of 3 tasks
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 this PR does
With a positive --rate-limit-tokens-per-second on the released SMG v1.12.0, are queued requests admitted in FIFO order with the --queue-timeout-secs wait bound, or does that behavior exist only on unreleased main as the docs currently state?
Why we need it
Source change: smg-project/smg@f3723c5
Documentation targets the pinned published stable release.
Commit 112 (smg#2733, f3723c52, 2026-10-02) rewrote model_gateway/src/middleware/token_bucket.rs so the FIFO waiter handoff applies in both refill modes; it is a first-parent ancestor of the pinned v1.12.0 release (tag v1.12.0 = dcc5cb0b, 2026-10-08), and the release checkout confirms it: .nightly-source/model_gateway/src/middleware/token_bucket.rs:16-27 ('FIFO waiter handoff: returned (and refilled) tokens are granted directly to the oldest waiter... new arrivals cannot barge past the queue'; only the front waiter arms a refill timer) and middleware/concurrency.rs:214-238 (queued requests wait via TokenPermit::acquire_timeout bounded by queue.queue_timeout, then 503 admission_queue_timeout). Stale docs, the only page carrying this framing (repo-wide greps for '2733', 'oldest waiter', 'FIFO'): src/lib/content/concepts/reliability/rate-limiting.md:98 'The FIFO handoff applies in both refill modes; in v1.11.0 and earlier it applies only in the default mode...' followed by :111-112, the note titled 'Changed after v1.11.0' ending 'On main after the v1.11.0 release (smg-project/smg#2733), refilled tokens go to the oldest waiter like returned ones' — at the pinned v1.12.0 release this frames released behavior as unreleased current-main, so a v1.12.0 operator reading the page concludes their release still has the old poll/1-in-N queue behavior. The fix is to re-anchor the note to v1.12.0 in the page's existing caveat style (the docs already use 'newer than v1.12.0 / In v1.12.0 and earlier' elsewhere, e.g. circuit-breakers.md:122), keeping the v1.11.0-and-earlier poll behavior as the historical caveat and dropping the commit link per docs style. This is one independent concern: one commit's released behavior, re-anchored where the one page states it. Everything else on the page was re-verified against the release and stays true at v1.12.0: --max-concurrent-requests default -1/disabled, queue_size 100, queue_timeout_secs 60 (main.rs:946-958), shed codes admission_queue_full/admission_queue_timeout with Retry-After (concurrency.rs:36,196-238), metric names (observability/metrics.rs:215-227) — and the edit must NOT touch the off-by-default/-1 contract, which the post-release smg#2927 (closed release-deferred PR #152) changes only after v1.12.0. Other refill mentions need no edit: reference/configuration.md:489 and getting-started/reliability-controls.md:36 describe only the in-flight-exceeds-cap effect (unchanged), reference/metrics.md:236's FIFO sentence is unconditionally correct. Dedup: merged PR #99 (marker rate-limiting:refill-queue-fifo-order, same source commit) documented the behavior correctly as main-only before the release; this follow-up only reconciles the release anchor and does not repeat its content; no other open or closed PR mentions #2733 or queue ordering; no open PR holds rate-limiting.md (open #168/#166/#164/#160 hold other files). Edit is a few sentences, far under 999 lines.
Scope: rate-limiting / refill-queue-fifo-release-anchor. Other concerns are deferred.
How to test
git diff --checkandpnpm checkand the productionpnpm build.Checklist