fix(maintenance): stop the scheduled mental-model refresh from enqueueing duplicates (#3210) - #3212
Merged
Merged
Conversation
…eing duplicates (#3210) The maintenance loop runs in every API/worker process with no leader election, so N processes were N schedulers making the same due-and-stale judgment each interval. The in-flight guard that should have prevented a second refresh lives in the discovery routine `mental_models_with_cron()` — a *read*, so every process saw the same "nothing in flight" snapshot and inserted its own operation. A few hundred due models became thousands of queued refresh ops, which occupy claim slots, inflate queue-depth (an autoscaler input) and delay unrelated tenants. The check now rides on the INSERT itself: with `submit_async_refresh_mental_model(skip_if_in_flight=True)` the operation row is only materialised `WHERE NOT EXISTS` a pending/processing `refresh_mental_model` op for the same `(bank_id, mental_model_id)`, so the check cannot be separated from the write. The submit also takes the existing `FOR NO KEY UPDATE` bank-row lock that `dedupe_by_bank` uses (#1842) — no extra round-trip — so two simultaneous submits serialize instead of both passing the READ COMMITTED snapshot. Only the cron scheduler opts in; explicit user-triggered refreshes (HTTP, MCP, consolidation-triggered) still queue unconditionally and are never swallowed.
nicoloboschi
force-pushed
the
fix/3210-mm-refresh-duplicate-enqueue
branch
from
August 6, 2026 15:26
f571601 to
57d310c
Compare
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.
Fixes #3210
Why the duplicates happened
The in-flight check the issue asks for already exists — but in the wrong place to
be safe.
mental_models_with_cron()excludes models that have a pending/processingrefresh_mental_modelop, and that exclusion is a read. The maintenance loop runsindependently in every API/worker process with no leader election, so every process
evaluated the same snapshot ("nothing in flight, due, stale") and inserted its own
operation. That is the ~8× amplification reported: one wave per process, not one wave
per stalled interval.
The fix
The check now rides on the INSERT.
submit_async_refresh_mental_model(skip_if_in_flight=True)inserts the operation row only
WHERE NOT EXISTSa pending/processingrefresh_mental_modelop for the same(bank_id, mental_model_id)— one statement, sothe check can't be separated from the write:
Nothing returned ⇒ a refresh was already scheduled ⇒ the caller gets that operation's
id with
deduplicated=True, and the loop logs it as "already in flight". The submitalso takes the
FOR NO KEY UPDATEbank-row lock thatdedupe_by_bankalready uses(#1842) — it is a modifier on the bank-exists SELECT that runs anyway, no extra
round-trip — so two simultaneous submits serialize rather than both passing the READ
COMMITTED snapshot.
No new table, no schema change, no scheduler coordination.
Only the cron scheduler opts in. Explicit user-triggered refreshes (HTTP, MCP,
consolidation-triggered) keep today's behaviour — an on-demand refresh is never
swallowed.
Tests
test_concurrent_scheduled_submits_queue_one_refresh— two schedulers enqueue thesame due model with completions stalled → exactly one queued op, and the loser
reports the winner's id. Verified it fails without the fix (
assert 2 == 1).test_user_triggered_refresh_is_not_deduplicated— an explicit refresh still queueswhile a scheduled one is in flight.
test_mental_model_scheduled_refresh.py,test_mental_model_delta.py,test_consolidation_submit_atomic_dedup.py,test_async_submit_bank_not_found.py,test_maintenance_*,test_worker.py,test_operation_status.py,test_admin_backup_restore.pyandtest_migration_shape.pypass locally;lint.sh+tyclean.Not changed
MemoryEngine.initialize()'sif self._initialized: returnguard is notconcurrency-safe, so two concurrent calls could register two loops in one process —
which would explain the issue's "same scan twice within seconds". Left alone as a
separate concern; with the enqueue now idempotent, an extra scan queues nothing extra.