Skip to content

fix(metrics): make pool_name mean one thing across every gate series - #372

Merged
shimib merged 2 commits into
mainfrom
fix/gate-pool-label
Jul 29, 2026
Merged

fix(metrics): make pool_name mean one thing across every gate series#372
shimib merged 2 commits into
mainfrom
fix/gate-pool-label

Conversation

@shimib

@shimib shimib commented Jul 29, 2026

Copy link
Copy Markdown
Member

Fixes #369.

Problem

Five series answer "why is this queue throttled?". They carried three different pool_name values, so none of them joined:

Series pool_name was Other labels
async_gate_metric_value / _threshold the gate's pool param — the InferencePool none
async_dispatch_budget, async_gate_decisions_total configMap[queueID].WorkerPoolID"" when the queue omitted worker_pool_id queue_id, queue_name
async_broker_backlog, async_pool_worker_limit the request channel's pool — "default" queue_id, queue_name

Two separate defects:

  1. A plain bug. NewRedisSortedSetFlow defaults worker_pool_id to "default" for the request channel, then stores the raw config in configMap:

    workerPoolID := cfg.WorkerPoolID
    if workerPoolID == "" { workerPoolID = "default" }
    ch := pipeline.RequestChannel{ ..., WorkerPoolID: workerPoolID }
    r.configMap[cfg.ID] = cfg   // WorkerPoolID still ""

    So one queue emits pool_name="" on two of its series and "default" on the rest. Pub/Sub was never affected — it reads the pool off the already-defaulted request channel.

  2. A label-semantics collision. pool_name on the gate gauges is the InferencePool the gate measures; everywhere else it is the async worker pool the series belongs to. One label name, two concepts — and the gate gauges carried no queue_name to fall back on, so there was nothing to join on either. In the multitenant guide these happen to be equal strings, which is why it went unnoticed.

Change

One source for the label.

  • pipeline.GateConfig gains an Owner (queue_id, queue_name, worker_pool_id), json:"-" — stamped by the caller, never deserialized from user config. The three construction paths set it (pkg/redis/sortedset_impl.go, pkg/pubsub/pubsubimpl.go, pkg/server/runner.go), and GateFactory.CreateGate propagates it through all three recursive gate types (composite, wait-on-refuse, tier-priority-admission) — the inner configs round-trip through JSON, so this has to be explicit.
  • The gate gauges carry the full queue triple, so they join with async_dispatch_budget and everything else on on(queue_id, queue_name, pool_name). A pool-level gate has no single queue, so it leaves the queue labels empty and is keyed by pool_name alone.
  • The InferencePool is not lost — it moves to its own inference_pool label, which is what the pool param always meant. Two worker pools gating on the same InferencePool are now distinguishable; before, they overwrote each other's gauge.
  • Normalize WorkerPoolID before storing it, so configMap and the request channel cannot disagree. This is the issue's own suggested minimum, and it is still worth doing on its own — several other reads go through configMap.

Chose GateConfig.Owner over changing the GateFactory interface: CreateGate(cfg GateConfig) (Gate, error) is public API in a separately tagged module, and a new field is additive.

Docs

Both gauges were missing from the README metrics table entirely. Added them, spelled out that pool_name always names the async worker pool and inference_pool the queried InferencePool, documented the (previously undocumented) pool param of prometheus-query, and added the PromQL for a "why is the gate closed?" panel:

llm_d_async_async_gate_metric_value - on(queue_id, queue_name, pool_name) llm_d_async_async_gate_metric_threshold

Tests

  • pkg/async/inference/flowcontrol: the owner reaches a metric gate directly, and survives all three recursion sites — composite → wait-on-refuse → metric gate, and tier-priority-admission's inner saturation gate. Updated the [Feature]: Metrics #217 gauge test for the new label set.
  • pkg/redis: a queue config with no worker_pool_id lands in configMap as "default", and agrees with its request channel.

Compatibility

Breaking for dashboards on async_gate_metric_value / async_gate_metric_threshold. They gain three labels, and pool_name changes meaning from the InferencePool to the worker pool. A query that selected {pool_name="my-inference-pool"} should now select {inference_pool="my-inference-pool"}; one that only aggregated is unaffected. The two gauges shipped recently (#339, v0.8.0) and are not referenced by any chart or dashboard in this repo.

Queues that omitted worker_pool_id will see async_dispatch_budget and async_gate_decisions_total move from pool_name="" to pool_name="default" — the value their sibling series already reported.

Known limitation, unchanged by this PR: two metric gates inside one composite still share a series unless their pool params differ.

Note on verification

There is no Go toolchain on this machine, so I could not run make test locally — and this repo's CI does not run unit tests either (.github/workflows/pre-commit.yml runs make test-integration, helm lint and helm unittest; make test is never invoked). The new and updated tests are type-checked by go vet and golangci-lint in pre-commit, but have not been executed. Flagging it rather than implying otherwise. Adding a make test step to that workflow looks worth doing separately.

The five series an operator needs to answer "why is this queue throttled?"
carried three different pool_name values, so none of them joined:

  - async_gate_metric_value/_threshold labeled pool_name with the gate's
    'pool' param, i.e. the InferencePool being queried, and carried no
    queue labels at all;
  - async_dispatch_budget and async_gate_decisions_total read pool_name
    from configMap, which stored the raw queue config — so a queue that
    omitted worker_pool_id got pool_name="" ...
  - ... while the request channel, async_broker_backlog and
    async_pool_worker_limit all said "default" for that same queue.

Give them one source. GateConfig gains an Owner (queue id, queue name,
worker pool) that the Redis, Pub/Sub and pool-gate construction paths
stamp before calling CreateGate and the factory propagates through its
recursive gate types (composite, wait-on-refuse, tier-priority-admission).
The gate gauges are labeled from it, so they join with the rest of the
queue's series on the queue triple; a pool-level gate leaves the queue
labels empty and is keyed by pool_name alone. The InferencePool a gate
queries is not lost — it moves to its own inference_pool label, which is
what the 'pool' param always meant. Two worker pools gating on the same
InferencePool are now distinguishable, which they were not before.

Separately, normalize WorkerPoolID to "default" before storing the queue
config, so configMap and the request channel cannot disagree.

Documents both gauges in the README metrics table (they were missing),
what each label means, the 'pool' param of prometheus-query, and the
PromQL for a "why is the gate closed?" panel.

Signed-off-by: Shimi Bandiel <shimib@google.com>
Signed-off-by: Shimi Bandiel <shimib@google.com>
@shimib
shimib merged commit 955feb3 into main Jul 29, 2026
8 checks passed
@shimib
shimib deleted the fix/gate-pool-label branch July 29, 2026 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The three gate series carry three different pool_name values, so none of them join

2 participants