Repository navigation
feat(api): keep the resource-state read model current on writes - #332
Open
AlexAxthelm wants to merge 2 commits into
Open
AlexAxthelm wants to merge 2 commits into
AlexAxthelm wants to merge 2 commits into
Conversation
…-766) Attaching or creating sources, re-ordering a field's sources, and merging now recompute the affected resources' read-model rows in the same transaction. Each write locks its resources first (in id order for merges) and re-reads them after locking: locking later lets two writers deadlock upgrading the foreign-key locks their inserts take, and a write that waited must see a merge that committed meanwhile. Nothing reads the table yet, so answers are unchanged; each write pays ~85 ms at 50K resources for the refresh (~16 ms with #322's index). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new merge locks can deadlock with concurrent merge-candidate creation because resource lock ordering is inconsistent.
1 open finding
What changed in this PR
Keeps the resource-state read model current through API writes, building on #331 without enabling read paths yet.
Changes:
- Locks affected resources before writes and refreshes state in the same transaction.
- Adds mutation, concurrency, rebuild, and query-label coverage.
- Documents write-path requirements and refresh overhead.
| File | Description |
|---|---|
| deployments/api/tests/observability/test_query_name_actions.py | Checks refresh labels and label restoration. |
| deployments/api/tests/db/read_model/test_state.py | Tests legacy values and rebuild-time writes. |
| deployments/api/tests/db/read_model/test_mutations.py | Checks mutation consistency, rollback, and lock ordering. |
| deployments/api/tests/db/read_model/test_concurrency_postgres.py | Tests concurrent PostgreSQL writes. |
| deployments/api/tests/db/read_model/scenarios.py | Adds a shared rebuild-with-writes scenario. |
| deployments/api/tests/db/read_model/dataset.py | Clears write-generated state during fixture setup. |
| deployments/api/src/stitch/api/db/read_model/state.py | Documents write-first locking. |
| deployments/api/src/stitch/api/db/og_field_source_actions.py | Locks attachment targets and refreshes state. |
| deployments/api/src/stitch/api/db/og_field_resource_actions.py | Adds locking and refreshes for reorder and merge. |
| deployments/api/READ_MODEL.md | Documents maintenance rules and write overhead. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
FOR UPDATE conflicts with the key-share lock that inserting any row referencing a resource takes. Creating a merge candidate for [B, A] holds key-share on B, then waits on A, while a merge of [A, B] holds A and waits on B: Postgres aborts one. FOR NO KEY UPDATE (the lock an update of repointed_id already takes) still makes writers queue but never conflicts with key-share, so no insert path needs to lock first to stay safe. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AlexAxthelm
marked this pull request as ready for review
October 15, 2026 15:53
This was referenced Oct 15, 2026
Draft
Closed
This branch was successfully deployed
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.

Third PR of splitting #324 into smaller PRs for ease of review. Builds on #331: the various write actions now keep the read-model table current, so once it has been rebuilt it stays correct as resources are operated on (merge, attach, etc.). This is where the read model starts touching existing behavior: every write that can change a list answer now takes a row lock and does a bit more work.
Note that this imposes a small performance penalty over
mainbaseline at this point, since we're adding some small complexity to the write actions, but not using any of the read paths yet.Summary
Third PR in the STIT-766 read-model stack (builds on #331). Every API write that can change a list answer now recomputes the affected resources' read-model rows in the same transaction, so the table and the data commit together.
Which writes
createwith sources, attach, and create-and-attach. All three add memberships through one helper (_attach_source_models), which now refreshes the resource.set_field_source_priority).apply_resource_merge): the original resources lose their rows (they're no longer listable) and the new resource gets rows built from their combined sources.Locking first (the behavior change reviewers should look at)
lock_resources:SELECT ... FOR UPDATE, in ascending id order).Cost
The refresh adds ~85 ms per write at 50K resources (labeled
read_model.refreshin query timings). Almost all of it is finding the resource's memberships, becauseog_field_membershipshas no index onresource_id. The(resource_id, source_pk)constraint in #322 provides one, which brings it to ~16 ms per resource (~24 ms for a three-resource merge). Until #4 lands that cost buys nothing, so #3 and #4 should merge close together.Docs
deployments/api/READ_MODEL.mdgains:The stack
Related issues
Part of: STIT-766
Related: #322 (index that cuts the per-write cost)
Replaces: #324 (split for review)
Testing
tests/db/read_model/test_mutations.py: each write path (create, attach, create-and-attach, re-order, merge, merge-candidate approval, an in-place source value edit) leaves the table equal to a from-scratch recompute, and each recompute takes the lock before reading anything.tests/db/read_model/test_concurrency_postgres.py(Postgres only):tests/observability/test_query_name_actions.py: write actions also emitread_model.refresh, and the refresh's label doesn't leak into the write's own queries.tests/db/read_model/test_state.py: writes to a resource holding a value today's model rejects still succeed.make checkpasses.Checklist
AI use: Claude Code split this out of #324 and wrote most of the code, tests and docs. I reviewed it before committing. Verified by the tests and the deliberate breakage check above.
🤖 Generated with Claude Code