## Problem The per-chain write lock exists so that the head a writer reads for `previous_hash` is the head it extends. Inside a caller's open transaction it cannot do that, and the consequence is that chain rows are dropped under ordinary concurrency. `writeChainedRow()` acquires the lock, reads the head, signs, inserts and releases in its own `finally` — all inside the caller's still-open transaction. So the lock is released before the row is visible to anyone else. Worse, `getHeadHash()` is a snapshot-bound read: under `REPEATABLE READ` each caller reads the head from the snapshot its own transaction opened, so two callers whose transactions began before either committed read the same head however the lock is scheduled. Serialization on that path is not merely lost, it is unavailable. **What does not happen is chain corruption.** The `chain_previous_hash` unique key on (`chain`, `previous_hash`) catches exactly this and does its job: the second row never lands, the chain never forks, and integrity verification never reports a break it should not. That constraint is working as designed and this issue does not change it. What happens instead is that the write **fails**, and the audit row is lost. Two shapes, both ending the same way. If the first caller commits, the second gets a duplicate-key error and its row is dropped: a warning and a climbing dropped-under-contention counter in the best-effort path, an aborted host transition where a consumer re-throws. And because a unique-index insert must consider the other transaction's uncommitted row, the second caller does not fail immediately, it **waits** for the first to resolve, so a long-running caller transaction stalls every concurrent chain write behind it and can turn them into lock-wait timeouts. If the first caller rolls back instead, the second's insert succeeds and is correct, so that case self-heals. For a log whose value is completeness, losing rows under normal concurrent load is the defect, and the mechanism meant to prevent it is inoperative on precisely the path where consumers write most. **The defect is scoped to writes that ask this class for the lock.** Every chain write the module makes itself — the five lifecycle events in `ChainArchiver` and the restore event in `SegmentRestorer` — passes `acquire_lock: FALSE`, because the caller already holds the per-chain write lock, and in each case it took that lock before opening its transaction and holds it across the commit. Those writes are serialized exactly as intended and nothing below applies to them. What is affected is the path a consumer uses: `AuditTrailInterface::event()` and the PSR-3 logger, both of which ask the writer to take the lock itself. That scoping is also what makes the fix cheap to place. Both consumer paths discard the writer's return value, while the internal callers consume the returned row id synchronously, to bind `segment_*_event_id` and re-sign the segment's lifecycle HMAC. Deferring on `acquire_lock === TRUE` therefore defers exactly the writes that have no id to lose. ## Proposed resolution Handle it inside `audit_trail`, transparently. There is one mechanism and one setting, and the setting only ever offers a control where there is a real choice to make. **The mechanism: chain in the post-transaction callback.** When a write lands inside an open transaction, do not chain it there. Register a post-transaction callback and chain when the caller's outermost transaction resolves. The callback fires after the root commit has run and the stack is empty, so the head read is committed reality and the per-chain lock serializes as designed, taking one lock hold and one head read for the whole batch rather than one per row. Nesting needs no bookkeeping: the callback fires once, at the outermost resolution, never on a savepoint release. It reports success both for a committed transaction and for one voided by a DDL statement, each of which did commit the caller's work. **The setting: where the pending row waits.** Site-wide, three values. Writes made outside a transaction are unaffected by all of them and keep today's behaviour, as are the module's own writes that carry their own lock. - **outbox** (default): the row is written **inside** the caller's transaction, into the `audit_trail_outbox` table, then chained by the callback and deleted. Atomic with the fact it describes and rolled back with it; because the outbox insert happens inside the transaction it can still fail there, so a consumer that needs a failed chain write to abort its own transition keeps that ability. It survives a crash between the commit and the flush, so cron can finish what the callback did not. Costs one extra write and a delete per row, offset by the lock and head read amortising across the batch. - **memory**: the pending row is held in memory rather than a table. Same trigger, same flush, no extra writes, but a fatal between the commit and the flush loses the rows, and nothing was written inside the transaction, so there is nothing that could have aborted the caller. - **inline**: the current behaviour, kept as an explicit opt-out rather than a resting place. It carries the defect described above, and choosing it should be a deliberate acceptance of that. Those are the stored values. The form does not use those words. The person choosing is a site builder, not the author of this issue, so the choice is presented as where an audit entry waits when it happens in the middle of a larger change, and each option is described by what it costs and what it can lose, in plain language. Same for the two things this adds to the status report: an entry waiting to be recorded and an entry lost under load both have to be legible to someone who has never read this page. **One conditional control, under memory only.** A rolled-back transaction's rows are discarded by default: `outbox` cannot do otherwise, since the staged rows die with the transaction, and `inline` cannot either. `memory` is the only value where keeping them is possible, because the buffer sits outside the database's transactional fate, so that is the only value for which the form offers the choice. When it is on, each such row must be marked in its permanent bucket as describing work that was rolled back, or the chain asserts transitions that never happened. The premise behind discarding by default is that a consumer which cares about a failed attempt records it in a transaction that commits, rather than relying on the remains of one that did not. ## What an outbox has to get right An outbox is not automatically safe in a tamper-evident log, and one of these is not optional. - **A row waiting to be chained has no `previous_hash`, so it cannot carry the chain HMAC.** That leaves a window in which the record exists in mutable, unsigned form, and the chain would later attest faithfully to an altered payload. Sign the content at staging time with a hash binding the payload but not the chain link, and have the flush verify it before chaining. This is what makes the outbox safe to have. - **The chain lags the fact** until the flush runs, so verification has to treat an undrained tail as pending rather than missing, and the viewer has to account for it. - **Ordering is explicit work.** The flush chains in event order, which across concurrent requests is only as good as the captured timestamp, so `created` at event time matters here too. Across transactions the two clocks separate: `id` is still handed out in flush order, so it stays monotonic and a deferred row can never land behind one already written, but `created` can now run backwards against `id`, by at most the length of the longest transaction. The auto-archiver files rows into buckets by `created` and records each bucket as an id range, so two buckets' ranges can overlap. That resolves itself rather than needing a guard: `runCoverage()` mints through `ensureSegmentCoverage()`, which treats existing segments as obstacles and fills only the uncovered gaps, so the second range mints nothing where the first already covered, and the outcome is a single segment straddling the bucket edge. Retention clocks off `to_created`, the newest row in the segment, so nothing is purged before its own window. It needs a test, not a change. - **A stalled flush grows the outbox without bound**, so its depth belongs on the status report beside the dropped-row counter. ## Why the default moves off inline, and what that costs on upgrade `inline` is the defect, not a third opinion about it, so leaving it as the default would ship a known-broken default and make the fix something a site has to discover. The fix goes on by default. The upgrade cost is smaller than it first looks. Neither `KernelTestBase` nor `BrowserTestBase` wraps a test in a transaction: they isolate by per-test table prefix and drop the tables at teardown. A test that saves a content entity is inside one, because `SqlContentEntityStorage::save()` opens a transaction, but it **commits** it, so the callback fires and the row is there before `save()` returns. The entity, file and user-auth bridge tests keep passing unchanged, and no test in this module opens a transaction of its own. What does change is a test that opens a transaction itself and asserts on a chain row before resolving it. There are none here today, so this is a note for consumers rather than work in this issue: such a test has to move its assertion past the commit, or pin `inline` for the case. The release note should say so, because a consumer who has one will read it as a test failing rather than as a setting changing. A site that does select `inline` should not be left unaware of what it keeps. The writer already knows whether it is inside a transaction, so counting the writes it makes inline while in one costs nothing, and the status report is where this module already surfaces the rows dropped under lock contention. A site taking in-transaction writes on `inline` should be told there that its dropped-row counter has a structural cause, rather than reading it as ordinary contention. That count must exclude the writes that carry their own lock, or the module's own archiver traffic would drive a counter whose entire purpose is to report an unserialized write, and the warning would fire on a site that has no problem. ## What deferral costs Deferral cannot produce a row for a transition that rolled back, and cannot abort the caller either, because by the time it writes, the caller's work is already durable. A consumer that re-throws a write failure to abort its own transition loses that ability, which is the reason `inline` still exists, and the reason `outbox` is the default rather than `memory`: an outbox insert happens inside the caller's transaction and can still fail there. `memory` additionally has a window: the buffer is in memory, so a fatal between the commit and the flush loses the rows. `outbox` closes that window, at the price of one write and one delete per row. Because a missing row means something different depending on the mode, the mode that produced a row should be recorded with it, so a reader auditing the chain later can interpret what it sees. Recording it in the permanent bucket keeps the canonical payload, and therefore verification, uniform across modes and across chains written before this change. ## Known limitation: a rolled-back savepoint The post-transaction callback reports the outcome of the outermost transaction only. A caller that opens a savepoint, writes chain rows inside it, rolls that savepoint back and then commits its outer transaction leaves rows describing work that was undone: the deferred modes will flush them at the outer commit. Neither can tell, because the manager's public surface reports no savepoint resolution after the fact, and a released savepoint and a rolled-back one shrink the stack identically to an observer. `inline` is unaffected only because those rows roll back with everything else. Closing it needs the caller, which knows what it rolled back. A small opt-in API covers it: `audit_trail` hands out a marker for what has been buffered so far, and a caller that rolls a savepoint back asks for everything since that marker to be discarded. It stays opt-in, so the transparent path remains the default and only a caller that uses savepoints and rolls them back has anything to do. A signal from the database layer for savepoint resolution would remove the need for it, and is worth raising upstream rather than waiting on. ## Out of scope: chaining on a separate connection A fourth value, writing through a database connection that is not enrolled in the caller's transaction, would keep the one property the deferred modes cannot: a failed chain write can still abort the caller's transition, because the write happens at event time, and there is no crash window. It carries its own design load. The row commits on its own and therefore survives a caller rollback, so on that value the chain records attempts rather than outcomes, and such a row cannot simply be removed: the chain is append-only and every later row binds the previous one through `previous_hash`, so retracting one means appending a compensating entry that names it. It cannot be a default either, because a fresh install defines no second key and an unknown key is an unrecoverable error in the database layer. And it needs both halves of the read-modify-write routed through the second connection, since moving only the insert would still read the head from the caller's snapshot and produce a no-op that looks like a fix. That is a separate change with a separate risk profile, and holding this issue open for it would delay the fix for the defect. Split to #3620301. ## Implementation notes `created` must be captured when the event happens, not when the buffer is flushed. It is currently stamped inside `writeChainedRow()`, so buffering without moving that would give every row in one transaction the commit timestamp and lose the ordering the log exists to record. `addPostTransactionCallback()` throws when there is no active transaction, and the callback must be registered once per transaction rather than once per row, so the check and the registration have to sit together with nothing between them that could resolve the transaction. Nothing on the read side moves. A deferred row is simply not in the table yet, and lands at the head when it arrives, so the verifier, the segment reader, the archiver and the restorer are untouched. Tests are still the substantial part, for a different reason than the transaction one above. The mechanism has to be shown to work: a row absent inside the caller's transaction and present once it resolves, the batch chained in event order under a single lock hold and a single head read, a rolled-back transaction discarding its rows, the outbox draining on cron after an interrupted flush, and a hand-altered staged row refused at flush. The case that motivates the issue needs genuine concurrency: a single kernel test cannot produce two overlapping transactions, so neither the defect nor the fix is observable without two connections driven from one test, or parallel requests in a functional test. That is also why a suite of this size never caught it. ## Remaining tasks - Add the site-wide setting with the three values, `outbox` as the default and `inline` as the documented opt-out. - Implement the post-transaction callback flush, deferring on `acquire_lock === TRUE` with an open transaction, and capturing `created` at staging time. - Implement the outbox buffer in `audit_trail_outbox`, with the content hash verified at flush and cron finishing what the callback did not. - Implement the memory buffer, including the retained-rollback path and its permanent-bucket marker. - Add the keep-rolled-back-rows control, shown only when memory is selected. - Count and report in-transaction writes made on the inline setting, excluding the writes that carry their own lock, through the existing status-report requirements. - Put the outbox depth on the status report beside the dropped-row counter. - Record the producing mode with the row. - Add the opt-in marker and discard call for a caller that rolls back a savepoint, and raise the missing savepoint-resolution signal upstream. - Cover both deferred modes in the test suite, including the rolled-back caller, the cron drain, the coverage-bucket interleave, and a two-connection concurrency test. - Update `docs/architecture.md`, `docs/configuration.md` and `docs/consumers.md`, and the French translation. ## Release notes A chain write made inside a caller's open transaction could not be serialized by the per-chain write lock, because the head read is bound to the caller's transaction snapshot. Chain integrity was never at risk, since the no-fork unique key rejects the second row, but that row was dropped, and a long caller transaction stalled concurrent chain writes behind it. Such a write is now chained when the caller's transaction commits, so it succeeds instead of being lost, and by default it waits in the outbox table so a crash cannot lose it. The module's own lifecycle writes were never affected and are unchanged. A site that needs a failed chain write to be able to abort the caller can keep the previous behaviour, and with it the lost rows, by selecting the inline setting. A consumer whose test opens a transaction and asserts on a chain row before resolving it will need to move that assertion past the commit. This issue summary was drafted with the assistance of an AI agent (Claude). The analysis and the wording were reviewed by me before posting, and accountability for the content is mine.