fix(retain): thread ops into handle_document_tracking - #1358
Closed
youchi1 wants to merge 1 commit into
Closed
Conversation
Regression from vectorize-io#1325 (perf(db): eliminate ResultRow wrapping). Line fact_storage.py:312 was changed to pass ops=ops to delete_stale_observations_for_memories, but ops was never added to handle_document_tracking's signature, so every first-batch retain hit NameError: name 'ops' is not defined. Effect: every batch_retain task fails on the first-batch document tracking path. The poller schedules a retry, the retry fails the same way, and after the retry budget is exhausted the parent operation is marked failed. No memories get stored. The gateway's "hindsight: 1 retains (...) captured" log is misleading — that just means dispatch succeeded; the daemon's actual write path was raising all along. Fix is to give handle_document_tracking an ops parameter (defaulting to None to preserve the Oracle/junction-table fallback) and pass pool.ops from both call sites in orchestrator._streaming_retain_batch. Reproducible on any retain that triggers a first-batch document upsert. Verified against the daemon's structured log on a live deployment running main as of d8ec2d7.
Collaborator
|
fixed already in #1343 |
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
2 participants
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.
Bug
Regression from #1325.
fact_storage.py:312was changed to calldelete_stale_observations_for_memories(..., ops=ops)butopswas never added tohandle_document_tracking's signature, so every first-batch retain raises:Effect
Every
batch_retaintask fails on the first-batch document tracking path. Poller retries, fails again, parent operation gets marked failed. No memories get stored. The gateway log misleadingly showsretains capturedbecause dispatch succeeded.Fix
Add
ops=Nonetohandle_document_tracking's signature. Passops=pool.opsfrom both call sites inorchestrator._streaming_retain_batch. 3 lines changed.Verified
Reproduced on a live deployment running main as of d8ec2d7. Patch applied, retain plus background consolidation now succeed end to end (
created=6 updated=3on first run after fix).