Repository navigation
fix(accounts): a deterministic probe for the partition-lock test, and field tables for four document audit types - #1111
Merged
antoinetoussaint-byte merged 2 commits intoOct 13, 2026
Conversation
…e reader's transaction TestBringingAPartitionInStepLocksThePartitionNotItsName waited for provisioning to reach the target by reading pg_stat_activity on the reader's connection, which is inside the transaction that holds the target. A session collects the backend list, and each backend's statement, at its first read of pg_stat_activity in a transaction and shows that list until the transaction ends; only the wait event is read live. When the first probe ran before the provisioning backend had sent its statement, no later probe could match it, and the wait timed out after ten seconds. The probe now runs on a session of its own outside any transaction, so each read sees the backends as they are, and it matches provisioning sleeping between tries for the target or queued behind the reader for it, rather than any lock wait. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… a module AUDIT_METRICS.md now gives saas.document.read_unscoped, search_unscoped, confirmed and extractor_revised the field tables read, search and dead_letter_redriven have: each field's kind, requirement, bound and PII mark, taken from the audit registry, with the entry and idempotency-key requirements and the fields each type deliberately does not carry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
antoinetoussaint-byte
deleted the
fix/partition-test-race-and-audit-docs
branch
October 25, 2026 17:07
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.
Two items: a racy provisioning test, and four audit types that had no field tables.
1.
TestBringingAPartitionInStepLocksThePartitionNotItsNameraced its own probeIt failed on #1100's merge-queue ref with "provisioning never came to the target" after its 10 s
require.Eventually, and passed on the PR head.Diagnosis
The probe read
pg_stat_activityon thereaderconnection. That connection is inside the transaction that holds the target (BEGIN; LOCK TABLE ONLY ... IN ACCESS SHARE MODE). PostgreSQL collects the backend list, and the statement each backend is running, the first time a session reads them in a transaction. It shows that same list until the transaction ends. Onlywait_eventis read live from the backend's process entry.testify v1.12.1's
Eventuallyruns the condition once at the start, so the first probe goes out almost as soon as the provisioning goroutine launches. Provisioning has to get to its statement first: pool acquire,BEGIN,SET LOCAL ROLE, thenSELECT audit_events_provision_partitions($1). If the first probe beats that statement, the frozen list holds the provisioning backend with an older statement, or without it.query LIKE '%audit_events_provision_partitions%'can then never match, and the test fails after 10 s. If provisioning's statement wins, the list matches, the live wait event turns toPgSleepand the test passes. So the test passes or fails on which side wins at the start. A loaded runner shifts that.The provisioning SQL itself is not the race. In migration 34,
audit_partition_lockretriesLOCK TABLE ... NOWAITevery 50 ms against the partition's current name until its deadline. Provisioning can't get past the target while the reader holds it, so it stays there, sleeping, until the reader commits. It does not finish or move on early.Reproduction.
-count=50 -raceon the package's persisted test Postgres passed 50/50 locally. On a warm local pool, provisioning sends its statement within about 1.5 ms, before the first probe reads. A temporary probe log showed the first probe already matching, and in one run probe 1 at 0 and probe 2 at 1 inside the same transaction, which is the live wait event changing under a frozen list. To make the losing order certain, I added a 300 ms sleep before the provisioning call (temporary, not committed). With the old probe the test then failed every time: 465 probes over 10 s, all reading 0, "provisioning never came to the target". With the new probe and the same 300 ms delay it passed 10/10.Fix (a fix at the owner: the test's observation; no production change)
retentionSession, the same owner role as before) outside any transaction. Each read is its own transaction, so it sees the backends as they are. Every other probe in this file and in the retention tests already reads this way, throughtestPoolor a session outside a transaction.wait_event = 'PgSleep', and onlyaudit_partition_locksleeps), or queued behind the reader for it (reader pid = ANY(pg_blocking_pids(pid))). Before, it accepted any lock wait. The reader holds nothing but the target, so either arm means provisioning is at the target. While the reader holds the target, that state lasts from provisioning's first try until the reader commits. A single probe can miss it only in the sub-millisecond gap between two tries; the next probe, 20 ms later, lands in a 50 ms sleep.Mutation check: the test still guards its behaviour
I replaced
public.audit_partition_lock(regclass, timestamptz)in the test database for the length of the test and put the original definition back afterwards (pg_get_functiondef, restored int.Cleanup). None of this is committed.PERFORM audit_partition_lock_budget(deadline); EXECUTE format('LOCK TABLE %s IN ACCESS EXCLUSIVE MODE', target);. The new probe matches provisioning queued behind the reader. The test then fails 3/3 at the decisive assertion:provisioning did not wait for the follower, withcanceling statement due to lock timeout (SQLSTATE 55P03). The waitingLOCK TABLEfollowed the name to the partition the follower holds.55P03after the deadline).-race.2. Field tables for the four document types #1100 registered
module/AUDIT_METRICS.mddocumentedsaas.document.read,searchanddead_letter_redrivenfield by field. It now does the same forsaas.document.read_unscoped,saas.document.search_unscoped,saas.document.confirmedandsaas.document.extractor_revised. For each field it gives the kind, whether it is required, its byte bound and its PII mark. It also says which types require an entry and an idempotency key, which are observational and which transactional, and which fields each type deliberately leaves out (boundary,returnedandresult_counton the unscoped pair; the confirmed value onconfirmed).Every fact comes from
module/services/accounts/code/pkg/business/audit_registry.go: the definitions,documentFields,documentUnscopedReadFieldsand their comments. One more fact comes fromaudit_registry_test.go: a first attachment carries noparent_digest. Nothing generates this document. The generateddeployment/generated/communication.mdandevent-catalog.jsoncome from the registry but carry no field-level detail, and both were already regenerated by #1100. So the tables are written by hand, as the existing ones are.Gates run locally
go build ./...andgo vet ./pkg/infra/ ./pkg/business/(accounts): passgo test -race -count=50 -run TestBringingAPartitionInStepLocksThePartitionNotItsName ./pkg/infra/: passgo test -race -count=1 -run 'Partition|Provision|Retention|Audit' ./pkg/infra/: passgo test -race -count=1 ./pkg/business/: passgo test -race -count=1 ./pkg/infra/(the whole package): three retention tests inpostgres_audit_retention_test.go, a file this change does not touch, failed on timing. They wereTestRetentionWaitsForTheDefaultPartitionWithinItsTenSeconds(11.75 s against an 11 s bound),TestRetentionRunsAtReadCommittedWhateverTheDatabaseDefault(a store ping hit its deadline) andTestRetentionWaitsTenSecondsInAllForItsLocks(55P03). The machine's load average was about 97 during that run. Re-run on their own, all three pass.node module/tools/naming-gate.mjs check, plusrecords origin/main HEAD: passbash scripts/check-boundaries.sh: cleannode module/tools/commit-identity-gate.mjs check origin/main HEAD: passnode --test module/tools/agent-docs-links.test.mjs: passNot verified
codefly ci runlocally, and the local CLI is 0.1.181, while CI pins 0.1.185. CI is the record for the service lint phase.golangci-lintreports eight findings inpkg/infrathat predate this change and are in files it does not touch. I left them alone to keep this PR to its two items.🤖 Generated with Claude Code