fix: give memory embedding its own queue so it stops starving behind distillation - #305
Merged
Conversation
…distillation Embedding gates the entire curation pipeline: corpus_fully_embedded is false if any current retrieval document has a NULL embedding_pgvector, which sets comparison_complete=false project-wide and makes the judge's publish_new rung ineligible. It shared QUEUE_BATCH with session distillation and candidate decisions, served by one worker with concurrency 3, while worker-realtime sat idle. Measured on the stand: six ~0.3s embedding works sat in ready with queued runs for over twenty minutes behind a distillation session with 289 outstanding stages. QUEUE_REALTIME rather than QUEUE_NEAR_REALTIME, for isolation and not for the latency class its name suggests. Near-realtime is one FIFO with observation ingestion, and ReembedMissingEmbeddings bulk-enqueues up to 200 works per beat tick, so that choice would convert an embedding-provider slowdown into an ingestion outage - the same head-of-line bug with the roles swapped. QUEUE_REALTIME carried no tasks before this and has a consumer in both worker layouts; a brand-new queue would risk having none, since the deployed topology is gitignored and untested. process_candidate_decision_work_v1 deliberately stays on batch: it chains a judge chat call and an embedding, so it is long-running by design.
Owner
Author
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Bundle ReportBundle size has no change ✅ Affected Assets, Files, and Routes:view changes for bundle: engram-frontend-client-array-pushAssets Changed:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #305 +/- ##
=======================================
Coverage 91.09% 91.09%
=======================================
Files 223 223
Lines 23344 23344
=======================================
Hits 21266 21266
Misses 2078 2078 ☔ View full report in Codecov by Harness. |
The C4.3 harness starts exactly one worker and its whole subject is the memory embedding work: it waits for an active embedding lease, SIGKILLs the worker mid-embedding and asserts atomic recovery. It started worker-batch, which consumes only engram-batch, so once embedding moved to engram-realtime the work was never consumed and the test failed correctly. Point the built/started/killed/stopped service and the failure-log collection at worker-realtime, which consumes engram-realtime. ENGRAM_FAKE_PROVIDER_DELAY_MS moves with it, since it is what makes the embedding slow enough to catch mid-flight. No assertion is weakened, no timeout extended, and the test still kills exactly one worker. e2e_c43_atomic_memory_tests.py asserts the failure-log service list, so it moves with the constant it pins.
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
embed_memory_projection_work_v1moves fromQUEUE_BATCHtoQUEUE_REALTIME. One routingline, two tests.
Why
Embedding gates the entire curation pipeline.
corpus_fully_embedded(curation_shortlist.py:240) is false if any current retrieval document has a NULL
embedding_pgvector, which setscomparison_complete=falseproject-wide and makes thejudge's
publish_newrung ineligible.It shared
QUEUE_BATCHwith session distillation and candidate decisions, served by a singleworker at concurrency 3, while
worker-realtimesat idle:Measured on the stand tonight, right after deploying #304: six
memory_embeddingworks satin
readywithqueuedruns for over twenty minutes, behind a distillation session with 289outstanding stages. Run by hand they each completed in 0.286–0.598s. They were never
broken — they were never given a worker slot.
The effect is a priority inversion with a feedback loop: every write outcome creates a new
memory version, therefore a new unembedded document, therefore a new embedding work — so
curation continuously generates the work it is blocked on, and that work queues behind the
longest job in the system.
Why
QUEUE_REALTIMEand notQUEUE_NEAR_REALTIMENear-realtime is one FIFO queue with observation ingestion
(
process_observation_recorded,process_observation_work_v1), andReembedMissingEmbeddingsbulk-enqueues up to 200 works per beat tick (every 15 minutes).Prefetch does not reorder anything — an observation behind N embeddings waits for all N. At
0.3s each that is a ~20s ingestion delay; at the configured 30s embedding ceiling it is ~33
minutes. That is the same head-of-line bug being fixed here, with the roles swapped.
QUEUE_REALTIMEcarried no tasks before this (it appears inceleryconfig.pyonly at itsdefinition and in the
task_queuesdeclaration, never intask_routes) and already has aconsumer in both worker layouts. A brand-new dedicated queue was rejected deliberately: the
deployed topology lives in a gitignored
compose.server.ymlthat no test pins, so a queuewith no subscriber would silently stall embeddings entirely with no failure signal. Tracked
as B-010.
The name is a stretch for a background projection step. The queue is being used for its
isolation, not its latency class, and the routing table says so.
Not changed
process_candidate_decision_work_v1stays on batch. It chains a judge chat call plus anembedding (
WORK_TIMEOUT_SPECS[CANDIDATE_DECISION]:chat_calls=1,embedding_calls=1,soft 240s), so it is long-running by design; moving it would recreate the same inversion and
block the very embeddings this change frees. The asymmetry is the point — embedding is
chat_calls=0.Tests
test_embedding_projection_worker_is_registered_on_realtime_queue— the pre-existing routetest, renamed, all four original assertions kept with only the queue constant flipped.
test_embedding_projection_does_not_queue_behind_session_distillationtest_embedding_projection_does_not_share_a_queue_with_observation_ingestionThe last two pin the property rather than the constant, so a future edit that folds these
back together fails even if the queue names change.
Verified:
pytest -q engram/memory engram/core→ 2098 passed;ruff checkandruff format --checkclean. Route tests were run red before green.Limits
This fixes scheduling, not generation. Curation still mints one NULL-embedding document per
write outcome, so
comparison_completestill oscillates between a decision and its embedding— one unembedded document can still veto every decision in the project. That coupling is
B-009 and needs a design change (a grace window, or per-shortlist rather than per-corpus
completeness), not a routing change.