fix: upsert-before-delete-stale and per-service reindex lock#91
Merged
Conversation
Reindexing deleted a file's symbols before upserting the replacements,
leaving a window where search returned zero results for that file if a
query landed between the two calls or the process died mid-update.
Concurrent reindex requests for the same service also had no
serialization, so overlapping runs could race each other's
delete/upsert calls against Qdrant.
Point ids are deterministic (uuid5 of service/file/symbol/line), so
upsert the new/changed symbols first and only delete the ones that
dropped out of the new set. Add a per-service asyncio.Lock (keyed by
"code:{service}" / "history:{service}") around IndexPipeline and
GitHistoryPipeline's index_service so overlapping reindex requests for
the same service queue instead of interleaving.
Fixes #69
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Owner
Author
|
Verified locally, it all works. |
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
1 participant
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.
Summary
Fixes #69 — reindexing had two related races:
IndexPipeline.index_servicecalleddelete_by_filethenupsert_chunksas two separate Qdrant calls. A search landing between them (or a crash mid-update) would see zero symbols for that file./reindex,/reindex-history, and the MCPreindextool all spin up a fresh pipeline per call with nothing preventing two overlapping runs for the same service from racing each other's delete/upsert calls.Changes
server/store/qdrant.py: addedget_point_ids_by_fileanddelete_by_ids;upsert_chunksnow returns the ids it wrote.server/indexer/pipeline.py: for each changed file, upsert the new/changed symbols first, then delete only the ids that dropped out of the new set (point ids are deterministicuuid5(service:file:symbol:line), so this diff is exact) — no window where the file has zero indexed symbols.server/state.py: addedget_reindex_lock(key), a registry ofasyncio.Lockper key.server/indexer/pipeline.py/server/indexer/git_history.py: wrappedindex_servicebodies inget_reindex_lock(f"code:{service}")/get_reindex_lock(f"history:{service}")so overlapping reindex requests for the same service serialize (queue) instead of interleaving, regardless of entry point (HTTP or MCP tool). Different services still index fully concurrently.tests/test_pipeline.py: updated the two tests that asserted the old delete-then-upsert ordering, and added regression tests for upsert-before-delete-stale ordering and for same-service reindex serialization.Note: the lock blocks and queues overlapping requests rather than rejecting them — a second reindex call for the same service will simply wait for the first to finish rather than failing or racing it.
Test plan
uv run pytest— 265 passeduvx ruff check ./uvx ruff format --check .— clean