feat(apply): dual-write apply_operations row on apply create#214
Conversation
ApplyService.createStoredApply now writes one apply_operations row
alongside the applies row and its initial tasks, all in a single
transaction. This puts the per-deployment data shape in place ahead of
the operator claim-loop change that consumes it.
Storage layer:
- Add ApplyStore.CreateWithTasksAndOperations(ctx, apply, tasks,
operations) — the new atomic apply-create path.
- Refactor CreateWithTasks to delegate with nil operations so all the
lock + transaction code lives in one place.
- Extract insertApplyOperation(ctx, exec, op) so the in-tx caller and
the existing ApplyOperationStore.Insert share the same insert + dup-key
translation (ErrApplyOperationExists on the (apply_id, deployment)
unique constraint).
Service layer:
- createStoredApply builds one ApplyOperation mirroring the apply's
(Deployment, Target) and passes it through CreateWithTasksAndOperations.
- The plan already carries the resolved (deployment, target) pair from
plan-time ResolveDatabaseTarget, so we do not re-resolve at apply time;
re-resolution would silently change routing if config drifted between
plan and apply. The next PR (which makes plans deployment-agnostic for
multi-deployment configs) is the right place to wire ResolveDatabaseTargets
at apply time.
- The config layer still hard-blocks deployments maps with >1 entries,
so this slice is always length 1 today.
Tests:
- TestExecuteApplyQueuesRemoteApplyForScheduler now also asserts the
dual-written operation (Deployment / Target / State=pending).
- New integration tests in pkg/storage/mysqlstore/applies_test.go:
- CreateWithTasksAndOperationsCommitsAtomically — apply + task +
operation are all visible after commit; ApplyID is back-filled on
the caller-supplied operation struct.
- CreateWithTasksAndOperationsRollsBackOnOperationFailure — a duplicate
(apply_id, deployment) on the second operation insert rolls back the
whole transaction, leaving no orphan apply row.
Verification:
- go build ./...
- go vet ./...
- go vet -tags integration ./pkg/storage/mysqlstore/...
- go test ./pkg/api/... ./pkg/state/... ./pkg/storage/... ./pkg/schema/...
./pkg/webhook/... ./pkg/tern/...
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea1da1cef3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // routing. The data shape is what the operator claim-loop PR consumes | ||
| // once that gate is lifted and plans become deployment-agnostic. |
There was a problem hiding this comment.
/workspace/schemabot/AGENTS.md says not to add comments describing what changed and not to reference PRs in code comments. In this new comment, the behavior is explained in terms of the current gate and an “operator claim-loop PR” that will consume the data later; that will go stale and violates the repository guidance. Please rewrite this as a stable invariant for why exactly one operation is written here today, or drop the future-work details.
Useful? React with 👍 / 👎.
| // (deployment, target). Today's hard-block keeps this at one row; the | ||
| // operator claim-loop PR is what consumes them. |
There was a problem hiding this comment.
/workspace/schemabot/AGENTS.md says comments should not describe temporary/current implementation state or reference PRs. This test comment bakes in “Today's hard-block” and “operator claim-loop PR” context, so it will become misleading once the next apply path lands. Please rephrase it to describe the durable behavior being asserted, or omit the explanatory comment.
Useful? React with 👍 / 👎.
1 participant
Apply create now writes one
apply_operationsrow alongside theappliesrow and its initial tasks, all in a single transaction. Puts the per-deployment data shape in place ahead of the operator claim-loop change that consumes it. No behavior change at apply time today — the claim loop still drives offappliesrows.Table of contents
Storage API
CreateWithTasksnow delegates to it withniloperations, so the lock + transaction code lives in one place.insertApplyOperation(ctx, exec, op)is extracted so the in-tx caller and the existing publicApplyOperationStore.Insertshare the same insert + dup-key translation.Service wiring
createStoredApplybuilds oneApplyOperationmirroring the apply's(Deployment, Target)and passes it throughCreateWithTasksAndOperations. The config layer still hard-blocksdeployments:maps with >1 entries, so this slice is always length 1 today.Why not call ResolveDatabaseTargets at apply time
The plan already carries the resolved
(deployment, target)pair from plan-timeResolveDatabaseTarget. Re-resolving at apply time would silently change routing if server config drifted between plan and apply. The next PR (which makes plans deployment-agnostic for multi-deployment configs) is the right place to wireResolveDatabaseTargetsat apply time.Tests
TestExecuteApplyQueuesRemoteApplyForSchedulernow also asserts the dual-written operation (deployment / target / state=pending).pkg/storage/mysqlstore/applies_test.go:CreateWithTasksAndOperationsCommitsAtomically— apply + task + operation all visible after commit;ApplyIDback-filled on the caller-supplied operation struct.CreateWithTasksAndOperationsRollsBackOnOperationFailure— a duplicate(apply_id, deployment)on the second operation insert rolls back the whole transaction, leaving no orphan apply.Verification
go build ./...✅go vet ./...✅go vet -tags integration ./pkg/storage/mysqlstore/...✅go test ./pkg/api/... ./pkg/state/... ./pkg/storage/... ./pkg/schema/... ./pkg/webhook/... ./pkg/tern/...✅Out of scope
apply_operationsrows.deployments:maps.Rollback
Pure revert. New
apply_operationsrows written during soak are harmless — no reader yet — and can be left in place or deleted with a one-shot query.