fix(ingest): zero queue-depth gauge on all-queues-drained (review round 4)
- metrics: update_ingest_queue_depth guarded on `not by_queue`, which conflated
None (memory backend no-op) with {} (postgres, ALL queues drained). When every
queue drains at once, get_ingest_job_counts_by_queue returns {} and the
pre-zero loop was skipped, leaving a stale ghost backlog in the gauge. Guard on
`by_queue is None` only; add an all-drained regression test.
- procrastinate: note that INGEST_TRANSIENT_MAX_ATTEMPTS is snapshotted at
blueprint-build time (restart to pick up changes).
Deck #323.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
44f72839ed
commit
ce53e21ead
@@ -656,16 +656,20 @@ def update_ingest_queue_depth(by_queue: dict[str, dict[str, int]] | None) -> Non
|
||||
"""Set the per-tier-queue depth gauge from procrastinate job counts (#323).
|
||||
|
||||
``by_queue`` is ``{queue_name: {status: count}}`` (see
|
||||
``queue.procrastinate.get_ingest_job_counts_by_queue``). No-op on the memory
|
||||
backend (``by_queue`` is None).
|
||||
``queue.procrastinate.get_ingest_job_counts_by_queue``). No-op only on the
|
||||
memory backend (``by_queue is None``); an empty dict (postgres backend with
|
||||
every queue drained) still runs the pre-zero so the gauge reads 0.
|
||||
|
||||
Every managed queue is zeroed first: ``list_queues_async`` stops returning a
|
||||
queue once it has no jobs, so a queue that drained to empty drops out of
|
||||
``by_queue`` entirely. Without the pre-zero its gauge series would stick at
|
||||
its last non-zero value (ghost backlog in Grafana/alerts) instead of reading
|
||||
0. The live counts then overwrite the zeros for queues that still have work.
|
||||
``by_queue`` entirely (and when ALL drain, ``by_queue`` is ``{}``). Without
|
||||
the pre-zero its gauge series would stick at its last non-zero value (ghost
|
||||
backlog in Grafana/alerts) instead of reading 0. The live counts then
|
||||
overwrite the zeros for queues that still have work.
|
||||
"""
|
||||
if not by_queue:
|
||||
# ``is None`` not ``not by_queue``: an empty dict means "postgres, all queues
|
||||
# drained" and MUST still zero the gauge -- only None (memory) is the no-op.
|
||||
if by_queue is None:
|
||||
return
|
||||
# Lazy import to keep observability decoupled from the queue layer at module
|
||||
# load (and sidestep any import cycle); both names are public constants.
|
||||
|
||||
@@ -379,6 +379,9 @@ def _build_ingest_blueprint() -> Blueprint:
|
||||
# BaseRetryStrategy subclass is the documented extension point (and is
|
||||
# accepted at runtime by get_retry_strategy). The annotation is just too
|
||||
# narrow, hence the ignore.
|
||||
# Settings are snapshotted here at blueprint-build time (build_app, first
|
||||
# use), so a restart is needed to pick up INGEST_TRANSIENT_MAX_ATTEMPTS
|
||||
# changes -- intentional: the strategy lives for the App's lifetime.
|
||||
retry=TieredEscalationStrategy(
|
||||
max_transient_attempts=get_settings().ingest_transient_max_attempts
|
||||
),
|
||||
|
||||
@@ -39,3 +39,17 @@ def test_none_is_noop(metric_sample):
|
||||
assert metric_sample(
|
||||
_METRIC, {"queue": "ingest-fast", "status": "doing"}
|
||||
) == approx(2)
|
||||
|
||||
|
||||
def test_all_queues_drained_empty_dict_zeroes(metric_sample):
|
||||
# postgres backend with every queue drained → get_ingest_job_counts_by_queue
|
||||
# returns {} (list_queues_async drops empty queues). An empty dict is NOT the
|
||||
# memory-backend no-op: it must still zero every managed queue's gauge.
|
||||
update_ingest_queue_depth({"ingest-fast": {"todo": 9}})
|
||||
assert metric_sample(_METRIC, {"queue": "ingest-fast", "status": "todo"}) == approx(
|
||||
9
|
||||
)
|
||||
update_ingest_queue_depth({})
|
||||
assert metric_sample(_METRIC, {"queue": "ingest-fast", "status": "todo"}) == approx(
|
||||
0
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user