Skip to content

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 into
mainfrom
fix/partition-test-race-and-audit-docs
Oct 13, 2026
Merged

antoinetoussaint-byte merged 2 commits into
mainfrom
fix/partition-test-race-and-audit-docs

Conversation

@antoinetoussaint-byte

@antoinetoussaint-byte antoinetoussaint-byte commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Two items: a racy provisioning test, and four audit types that had no field tables.

1. TestBringingAPartitionInStepLocksThePartitionNotItsName raced its own probe

It 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_activity on the reader connection. 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. Only wait_event is read live from the backend's process entry.

testify v1.12.1's Eventually runs 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, then SELECT 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 to PgSleep and 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_lock retries LOCK TABLE ... NOWAIT every 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 -race on 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)

  • The probe runs on its own session (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, through testPool or a session outside a transaction.
  • The condition is narrower: provisioning sleeping between tries for the target (wait_event = 'PgSleep', and only audit_partition_lock sleeps), 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.
  • No timeout was changed and no assertion is retried.

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 in t.Cleanup). None of this is committed.

  • Lock by name, waiting (the behaviour before migration 34): 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, with canceling statement due to lock timeout (SQLSTATE 55P03). The waiting LOCK TABLE followed the name to the partition the follower holds.
  • NOWAIT retries under the name read once, no identity check: the test fails 3/3 the same way (55P03 after the deadline).
  • With the original function restored, the test passes 50/50 under -race.

2. Field tables for the four document types #1100 registered

module/AUDIT_METRICS.md documented saas.document.read, search and dead_letter_redriven field by field. It now does the same for saas.document.read_unscoped, saas.document.search_unscoped, saas.document.confirmed and saas.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, returned and result_count on the unscoped pair; the confirmed value on confirmed).

Every fact comes from module/services/accounts/code/pkg/business/audit_registry.go: the definitions, documentFields, documentUnscopedReadFields and their comments. One more fact comes from audit_registry_test.go: a first attachment carries no parent_digest. Nothing generates this document. The generated deployment/generated/communication.md and event-catalog.json come 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 ./... and go vet ./pkg/infra/ ./pkg/business/ (accounts): pass
  • go test -race -count=50 -run TestBringingAPartitionInStepLocksThePartitionNotItsName ./pkg/infra/: pass
  • go test -race -count=1 -run 'Partition|Provision|Retention|Audit' ./pkg/infra/: pass
  • go test -race -count=1 ./pkg/business/: pass
  • go test -race -count=1 ./pkg/infra/ (the whole package): three retention tests in postgres_audit_retention_test.go, a file this change does not touch, failed on timing. They were TestRetentionWaitsForTheDefaultPartitionWithinItsTenSeconds (11.75 s against an 11 s bound), TestRetentionRunsAtReadCommittedWhateverTheDatabaseDefault (a store ping hit its deadline) and TestRetentionWaitsTenSecondsInAllForItsLocks (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, plus records origin/main HEAD: pass
  • bash scripts/check-boundaries.sh: clean
  • node module/tools/commit-identity-gate.mjs check origin/main HEAD: pass
  • node --test module/tools/agent-docs-links.test.mjs: pass

Not verified

  • I did not run codefly ci run locally, and the local CLI is 0.1.181, while CI pins 0.1.185. CI is the record for the service lint phase.
  • Default golangci-lint reports eight findings in pkg/infra that predate this change and are in files it does not touch. I left them alone to keep this PR to its two items.
  • I could not reproduce the merge-queue failure itself. The diagnosis rests on PostgreSQL's per-transaction activity snapshot and on the forced-delay reproduction above.

🤖 Generated with Claude Code

antoinetoussaint-byte and others added 2 commits October 13, 2026 09:09
…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
antoinetoussaint-byte added this pull request to the merge queue Oct 13, 2026
Merged via the queue into main with commit 04364d3 Oct 13, 2026
25 checks passed
@antoinetoussaint-byte
antoinetoussaint-byte deleted the fix/partition-test-race-and-audit-docs branch October 25, 2026 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant