Move PostgreSQL transactions and the execution lease into pgunit - #301
Merged
Merged
Conversation
persistence/postgres/pgunit now owns Core's transaction mechanics: pooled read-write and snapshot transactions, and the execution lease with its dedicated connection, gate, ownership check, cancellation fence, close and five-second execution deadline. store runs every transaction through it. The connection choice is fixed at construction. store.New builds a pooled Store; store.NewExecution takes the lease and builds the execution writer whose Session and execution-only transactions run on the leased connection. Execution-only operations on a pooled Store fail with ErrExecutionAuthority. ExecutionLease and its Store() view are gone. persistence/postgres/pgtest is the shared test database helper: the oac_*_tests guard, migrations, and isolated databases for database-wide state. It replaces the per-package database setup in store, execution and sandbox/providers tests. Also fix the go vet copylocks warnings in the store dispatch fixtures and record the layering in services/core/IMPLEMENTATION.md.
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.
This is the first step of removing the
storepackage. The shared PostgreSQL transaction and execution-lease mechanics move toservices/core/internal/persistence/postgres/pgunit, where later domain adapters (persistence/postgres/<domain>pg) can use them without importing store.What changes
pgunit(new):Pool.Transaction: read committed, read-write.Pool.Snapshot: repeatable read, read-only.Lease: the advisory-lock connection, the serializing gate, the ownership ping, cancelling operations only between leased operations, close with the cleanup wait, and the 5 s execution deadline.pgtest(new, tests only):Open(t)for guarded, migrated test databases.OpenIsolated(t, …)for tests of database-wide state such as the lease or provider identity.store.Newbuilds a pooled Store.store.NewExecution(ctx, s)acquires the lease, and that lease is both the writer and the authority.store.ErrExecutionAuthority. There is no pooled fallback after lease loss or close.AppendTurnEventschecks the whole batch before any write or replay.store/execution_lease.goandExecutionLease.Store().pgx.Begin/BeginFuncin non-test store code; 31 files now go through pgunit.*pgunit.Leasefor execution operations, so authority becomes a type. This is recorded in the new Layering section ofIMPLEMENTATION.md.go vet ./services/core/...is clean; the copylocks findings are fixed.Minor behaviour changes
Worker.CheckOwnershipis bounded by the 5 s execution deadline.ErrExecutionAuthorityinstead ofErrInvalidInputor ad-hoc errors. No production caller reaches them.Checks
go build ./...,go vet ./services/core/...,check-names.pyand the markdown link test pass.go testagainst PostgreSQL over persistence, store, execution, cmd, sandbox, api, runtime* and tests: 1175 passed, 12 skipped. The skips need a native daemon, a real model or a Docker image; none was for lack of a database.A blind review (Claude subagent) found nothing material.
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.