>>> [!note] Migrated issue <!-- Drupal.org comment --> <!-- Migrated from issue #3591896. --> Reported by: [mably](https://www.drupal.org/user/3375160) Related to !9 >>> <h3 id="summary-problem-motivation">Problem/Motivation</h3> <p>This is a follow-up to #3591878 (which shipped the TTL-renewal pattern on the chain-write lock to keep long archive + restore operations from outliving the lock's 30s TTL).</p> <p>The renewal pattern is a workaround. The underlying issue is that the chain-write lock is held for the entire archive operation when it is only strictly needed for the brief chain-event emission at the end. The long row-loop in the middle does not produce any write that can conflict with concurrent chain writers, and the DB-level <code>UNIQUE(chain, previous_hash)</code> index prevents chain fork regardless. Narrowing the lock scope on <code>archiveSegment()</code> solves the TTL problem at the design level rather than patching around it, AND removes a real operational issue: while the over-broad lock is held, every chain-write op on the chain blocks or fails. On a large archive, that means user-facing audit log writes drop for tens of seconds.</p> <h3>Scope</h3> <p>This issue narrows <code>archiveSegment()</code> only. <strong><code>restoreLocked()</code> is NOT included</strong> -- see the "Why restore is not narrowed here" section below for the design constraint and the open follow-up.</p> <h3>Analysis</h3> <p><code>ChainArchiver::archiveSegment()</code> (via <code>writeNdjson()</code>) holds the chain-write lock for a data-dependent duration:</p> <ol> <li>Verify segment state (read-only, no lock needed).</li> <li>Acquire chain-write lock.</li> <li>Per-row loop: <code>writeNdjson()</code> writes one NDJSON line per row in [from_id, to_id], plus <code>hash_file('sha256')</code> on the whole file.</li> <li>Update segment row lifecycle stamps.</li> <li>Emit <code>segment_archived</code> chain event at the live head (one INSERT into <code>audit_trail</code>).</li> <li>Rename temp NDJSON to its final path.</li> <li>Release lock.</li> </ol> <p>Of these, only steps 4-5 actually need the chain-write lock for chain-integrity reasons. The row loop in step 3 operates on a past id window that does not intersect the head; concurrent logger writes land at the head with the current head's hash as their <code>previous_hash</code>, and the archive's NDJSON file captures the pre-loop snapshot. The two cannot fork. The <code>UNIQUE(chain, previous_hash)</code> index is the load-bearing protection.</p> <h3>Operational impact</h3> <p>While the chain-write lock is held by archive, every other code path on the same chain blocks or fails:</p> <ul> <li>Logger writes (<code>AuditTrailChainWriter::writeChainedRow()</code>): block up to 5s then drop with drop-counter bump and throw.</li> <li>Cron archive / purge / file-purge / transient-purge / orphan-heal / createBareSegment / transientPurgeSegment: block up to 30s then throw. <code>safelyRunPass</code> catches each but the chain is skipped for that cron tick.</li> </ul> <p>For a 1M-row archive (~100s), this is effectively a 100-second outage of all chain-write activity on the chain. With the TTL-renewal pattern in #3591878, the outage is preserved-by-design: renewal is what keeps the lock held that long.</p> <h3>Proposed resolution</h3> <p>Refactor <code>archiveSegment()</code> to acquire the chain-write lock only around the bounded Phase B (segment row UPDATE + segment_archived chain event INSERT + rename). The long Phase A (writeNdjson + hash_file + HMAC computation) runs without the lock. Concurrent-archive-of-the-same-segment race is caught by an optimistic <code>WHERE archived_at = 0</code> predicate on the Phase B UPDATE -- second caller sees affected_rows = 0, throws, rolls back, unlinks temp file.</p> <h3>Why restore is NOT narrowed here</h3> <p>Initially this issue proposed narrowing <code>restoreLocked()</code> with the same pattern. Audit review surfaced a fundamental constraint: <code>restoreLocked()</code> needs a single DB transaction wrapping the row-replay loop AND the segment_restored chain event INSERT AND the segment row UPDATE -- atomicity is required for chain integrity, and a partial restore (rows present but no segment_restored event) is indistinguishable from a tamper at the verifier level.</p> <p>Three placements were considered:</p> <ul> <li><strong>Lock acquired inside the transaction</strong>: Drupal's <code>DatabaseLockBackend</code> shares the same DB connection, so the semaphore INSERT joins the transaction. The InnoDB row-level lock on the semaphore row is held for the full transaction duration. Concurrent logger writes block at the DB level (below Drupal's controlled retry) and surface raw <code>lock_wait_timeout</code> errors after 50s, bypassing the drop-counter contract. PHP-side lock release runs early but has no effect on the DB-level mutual exclusion.</li> <li><strong>Lock acquired before the transaction</strong>: lock held for the full transaction duration anyway. Same operational behavior as #3591878's wide-lock pattern. No narrowing achieved.</li> <li><strong>Two transactions (T1 row replay, T2 chain event + segment UPDATE)</strong>: breaks atomicity. If T2 fails after T1 committed, rows are restored but the segment row still says <code>live_purged_at != 0</code>. Verifier flags this as a tamper. Operator retry hits PRIMARY KEY conflicts on the already-restored rows.</li> </ul> <p>None of the three preserves both atomicity and DB-level narrowing without additional infrastructure. The two-transaction option becomes viable with a per-segment <code>restore_in_progress</code> lifecycle stamp + verifier tolerance for that intermediate state + restore retry detection of "T1 already committed" -- a schema change and verifier update that is out of scope here.</p> <p><strong>Open follow-up:</strong> #3591904 (to be filed) -- tracks the restore-narrowing work with the schema + verifier design.</p> <h3>Related</h3> <ul> <li>#3591878 (parent: shipped the TTL-renewal workaround that this issue's archive half replaces with a proper scope fix; the restore half stays as-is for now).</li> <li>#3591871 (grandparent: introduced the renewal pattern on the coverage lock).</li> </ul> <h3>Remaining tasks</h3> <ul> <li>Refactor <code>archiveSegment()</code> to narrow the chain-write lock scope (Phase A no lock, Phase B with lock).</li> <li>Revert the <code>writeNdjson()</code> signature change (drop <code>$lock_name</code> and <code>$renewed_at</code> parameters, drop renewer calls in the loop).</li> <li>Keep <code>renewChainWriteLockIfStale()</code>, <code>CHAIN_WRITE_LOCK_TTL_S</code>, <code>CHAIN_WRITE_LOCK_RENEW_INTERVAL_S</code>: now used only by <code>restoreLocked()</code>'s row-replay loop pending the follow-up.</li> <li>Restore-side changes from #3591878 stay unchanged.</li> <li>Kernel test: long-archive plus concurrent logger write, verifier accepts the resulting chain.</li> </ul> <h3>API changes</h3> <p>None for external callers.</p> <h3>Data model changes</h3> <p>None.</p>