fix(admission): split denied_queue, which was two mechanisms in one label #1106

Merged
coilyco-ops merged 1 commit from aos/claude/queue-denial-split-1083 into main 2026-08-22 22:17:06 +00:00
Member

Milestone 17 phase 2, working #1083. This does not close #1083, so the slice is filed as #1105 and closed here rather than weakening the reference.

The finding

#1083's first question is which bound is actually rejecting. Read against the source, the recorded data cannot say: two different refusals increment denied_queue, and on the Discord path both go out with transport=discord.

  • ratelimit.go refuses at admission when MAX_PENDING is reached, so the turn never waits at all.
  • runSerialized refuses a turn that was already admitted, waited for a slot, and gave up at the queue timeout. It gets noticeQueueTimeout rather than a cooldown notice, which is the giveaway that it is a different thing.

Those are opposite findings. A backlog refusal is an admission-sizing problem. A slot-wait refusal is a turn that ran long, which #995's pool and #1076's iteration cap move and admission sizing does not. #1083 measured 7.3% on dowel and 11.7% on echo, and either mechanism produces those numbers. Its own text reaches for exactly this distinction and could not make it.

What changed

denied_backlog and denied_slot_wait replace denied_queue. That is the whole change: the cheapest thing that lets the next measurement answer its own question rather than repeat it.

TestABacklogRefusalAndASlotWaitAreSeparateOutcomes reads what sirens_echo.admissions actually recorded through a manual reader, rather than asserting what the code appears to record, and it fails against the old shared label with a slot wait was recorded as a backlog refusal: map[denied_queue:1].

What stays open on #1083

The post-deploy re-measurement, which needs the slot pool to reach the cluster first. Worth noting that the pool changes the prior: with one slot, sustained occupancy by long turns was the obvious mechanism, and with eight it is much less so, which is why the milestone put #1083 after #995.

closes #1105
Refs #1083, #976, #995, #1076

Milestone 17 phase 2, working #1083. **This does not close #1083**, so the slice is filed as #1105 and closed here rather than weakening the reference. ## The finding #1083's first question is which bound is actually rejecting. Read against the source, **the recorded data cannot say**: two different refusals increment `denied_queue`, and on the Discord path both go out with `transport=discord`. * `ratelimit.go` refuses at admission when `MAX_PENDING` is reached, so **the turn never waits at all**. * `runSerialized` refuses a turn that **was already admitted**, waited for a slot, and gave up at the queue timeout. It gets `noticeQueueTimeout` rather than a cooldown notice, which is the giveaway that it is a different thing. Those are opposite findings. A backlog refusal is an admission-sizing problem. A slot-wait refusal is a turn that ran long, which #995's pool and #1076's iteration cap move and admission sizing does not. #1083 measured 7.3% on dowel and 11.7% on echo, and **either mechanism produces those numbers**. Its own text reaches for exactly this distinction and could not make it. ## What changed `denied_backlog` and `denied_slot_wait` replace `denied_queue`. That is the whole change: the cheapest thing that lets the next measurement answer its own question rather than repeat it. `TestABacklogRefusalAndASlotWaitAreSeparateOutcomes` **reads what `sirens_echo.admissions` actually recorded** through a manual reader, rather than asserting what the code appears to record, and it fails against the old shared label with `a slot wait was recorded as a backlog refusal: map[denied_queue:1]`. ## What stays open on #1083 The post-deploy re-measurement, which needs the slot pool to reach the cluster first. Worth noting that the pool changes the prior: with one slot, sustained occupancy by long turns was the obvious mechanism, and with eight it is much less so, which is why the milestone put #1083 after #995. closes #1105 Refs #1083, #976, #995, #1076
fix(admission): split denied_queue, which was two mechanisms in one label
All checks were successful
ci / test (pull_request) Successful in 58s
ci / publish-echo-image (pull_request) Has been skipped
ci / publish-observed (pull_request) Has been skipped
ci / image-build (pull_request) Successful in 32s
2d483436b4
#1083 asks which bound is actually rejecting. Read against the source,
the answer is that the measurement cannot say, because two different
refusals increment denied_queue on the same transport.

* The rate limiter refuses at admission when MAX_PENDING is reached, so
  the turn never waits at all.
* runSerialized refuses a turn that was already admitted, waited for an
  execution slot, and gave up at the queue timeout.

Those are opposite findings. The first is an admission-sizing problem.
The second is a turn that ran long, which #1076's iteration cap and the
slot pool in #995 both move and admission sizing does not. The 7.3%
denial share on dowel and 11.7% on echo could be either, and no query
against the recorded data separates them.

So they get their own outcomes, denied_backlog and denied_slot_wait,
which is the cheapest thing that makes the next measurement able to
answer its own question rather than repeating it.

The re-measurement #1083 also wants has to run after the slot pool
reaches the cluster, so it stays open carrying that half.

Refs #1083, #976, #995, #1076

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Agent-Role: engineer
coilyco-ops deleted branch aos/claude/queue-denial-split-1083 2026-08-22 22:17:06 +00:00
Author
Member

Right change, and the test is the right shape. It also breaks the verification query on #1083, in the direction that reads as success.

Darren (director seat). This is the split my measurement on #1083 implied and could not make, so I am glad it is the code and not a note. Two things before it merges.

The trap

My verification instructions on #1083 name the old label. That comment says to re-run the split after the pool lands and expect denied_queue on http and mcp to go to zero at the same traffic. After this rolls, that query returns empty, and an empty result there reads as "zero denials, the pool fixed it" when it actually means "the label no longer exists."

That is the worst available failure for a verification step: it confirms the thing it was meant to test. Update the query on #1083 in the same change, to outcome IN ('denied_backlog', 'denied_slot_wait'), and I will not have to remember that I wrote the wrong one.

The distinction also sharpens the expected result rather than just preserving it. If the pool works, denied_backlog should go to zero and denied_slot_wait is the one that may survive, since a slot wait is a turn that ran long and admission sizing does not touch it. Your body already reaches this conclusion about the prior. Writing it down as the expected shape makes the re-measurement falsifiable rather than open-ended.

Historical continuity

The pre-roll series keeps denied_queue forever, and the new names start empty. So any before-and-after comparison has to span all three names, and anyone reading a chart across the roll sees one line stop and two start. One sentence in docs/sirens-echo-admission.md saying the old name is the same metric before 2026-08-22 costs nothing and saves the next person the confusion.

What I checked so you do not have to

No alerting goes quiet. SigNoz has exactly two configured alert rules, "Agent Proxy trace errors" and "Kai-server k3s log ingest stalled". Both are disabled and neither touches sirens_echo.admissions. So the rename cannot silence a rule that was watching for this. I did not audit dashboard panels, which is the remaining place an old label could be hiding.

What is good and worth keeping in the next one

TestABacklogRefusalAndASlotWaitAreSeparateOutcomes reads what the metric actually recorded through a manual reader, rather than asserting what the code appears to record, and you falsified it against the old shared label. That is the only way a telemetry change proves anything, and it is the thing #1103's histogram write would have been caught by.

Merge it after the #1083 query

Everything else here is sound and the change is as small as the finding allows.

## Right change, and the test is the right shape. It also breaks the verification query on #1083, in the direction that reads as success. **Darren (director seat).** This is the split my measurement on #1083 implied and could not make, so I am glad it is the code and not a note. Two things before it merges. ### The trap **My verification instructions on #1083 name the old label.** That comment says to re-run the split after the pool lands and expect `denied_queue` on `http` and `mcp` to go to zero at the same traffic. After this rolls, that query returns **empty**, and an empty result there reads as "zero denials, the pool fixed it" when it actually means "the label no longer exists." That is the worst available failure for a verification step: it confirms the thing it was meant to test. **Update the query on #1083 in the same change**, to `outcome IN ('denied_backlog', 'denied_slot_wait')`, and I will not have to remember that I wrote the wrong one. The distinction also sharpens the expected result rather than just preserving it. If the pool works, **`denied_backlog` should go to zero and `denied_slot_wait` is the one that may survive**, since a slot wait is a turn that ran long and admission sizing does not touch it. Your body already reaches this conclusion about the prior. Writing it down as the expected shape makes the re-measurement falsifiable rather than open-ended. ### Historical continuity The pre-roll series keeps `denied_queue` forever, and the new names start empty. So **any before-and-after comparison has to span all three names**, and anyone reading a chart across the roll sees one line stop and two start. One sentence in `docs/sirens-echo-admission.md` saying the old name is the same metric before 2026-08-22 costs nothing and saves the next person the confusion. ### What I checked so you do not have to **No alerting goes quiet.** SigNoz has exactly two configured alert rules, "Agent Proxy trace errors" and "Kai-server k3s log ingest stalled". Both are disabled and neither touches `sirens_echo.admissions`. So the rename cannot silence a rule that was watching for this. I did not audit dashboard panels, which is the remaining place an old label could be hiding. ### What is good and worth keeping in the next one `TestABacklogRefusalAndASlotWaitAreSeparateOutcomes` **reads what the metric actually recorded through a manual reader**, rather than asserting what the code appears to record, and you falsified it against the old shared label. That is the only way a telemetry change proves anything, and it is the thing #1103's histogram write would have been caught by. ### Merge it after the #1083 query Everything else here is sound and the change is as small as the finding allows.
Sign in to join this conversation.
No reviewers
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-gaming/sirens-echo!1106
No description provided.