Problem/Motivation

Follow-up to #3591896. That issue narrowed the chain-write lock on ChainArchiver::archiveSegment(), but restoreLocked() stayed wide-locked with the TTL-renewer pattern from #3591878. This issue completes the narrowing for restore.

During a long restore (1M rows, ~100s), every audit log write on the same chain currently drops because the chain-write lock is held for the full restore transaction. Issue #3591896 documented the root cause: the chain-write lock cannot be placed inside the row-replay transaction (Drupal's DatabaseLockBackend shares the DB connection, so the semaphore INSERT joins the transaction and holds an InnoDB row-level lock for the full transaction duration), and it cannot be placed before the transaction without freezing all chain writes for the full duration.

Proposed resolution: emit segment_restored event first

Restructure restore from one big transaction wrapping everything into three discrete steps. The chain-write lock holds for only ~50ms in Step 1; the long row replay in Step 2 runs without the lock; the final segment row UPDATE in Step 3 runs in autocommit without the lock.

Step 1 (autocommit, with chain-write lock, ~50ms):
  acquireChainWriteLock(chain)
  INSERT segment_restored event at chain head
  releaseChainWriteLock

Step 2 (one transaction, NO chain-write lock, up to ~100s):
  BEGIN
    for each NDJSON row line: INSERT at original id
    for each NDJSON ack line: INSERT into audit_trail_acknowledgment
  COMMIT

Step 3 (autocommit, ~10ms):
  UPDATE audit_trail_segment
    SET live_purged_at = 0,
        live_purged_event_id = ,
        lifecycle_secret_id = ,
        lifecycle_hmac = 
    WHERE id = :segment_id

Why the chain event itself can serve as the in-progress marker

The chain event from Step 1 is HMAC-signed with the operator's current secret and chained via previous_hash to the live head. It is tamper-resistant attestation that a restore was initiated. Its presence in the chain after the segment's original segment_live_purged event signals "restore in progress or complete" without needing a new schema column.

The verifier extension: for any segment with a segment_restored event whose id is greater than the segment's current live_purged_event_id, accept transitional states without flagging:

  • Rows in [from_id, to_id] may be absent, partially present, or fully present.
  • Segment row's live_purged_event_id may still point at the original purge event (pre-Step-3) or at the new restored event (post-Step-3).

Per-row tamper detection (HMAC + previous_hash) is unaffected. The new rule only suppresses the structural "rows present in a purged range" flag when the chain itself attests to a restore in flight.

Operational benefit

During a 1M-row restore (~100s):

  • Step 1 holds the chain-write lock for ~50ms. Concurrent logger writes briefly contend via Drupal's normal retry path.
  • Step 2 holds no chain-write lock. Concurrent logger writes proceed at the chain head with no contention.
  • Step 3 holds no chain-write lock; brief segment row UPDATE in autocommit.

Total chain-write-lock-held duration during the restore: ~50ms instead of ~100s. Matches the operational improvement #3591896 delivered for archive.

Failure modes

  • Step 1 fails: nothing committed, no chain event. Operator retries from scratch.
  • Step 1 succeeds, Step 2 fails (any row INSERT throws -- PK conflict, malformed envelope, transient-hash mismatch): row INSERTs roll back atomically (Step 2 is one transaction). Chain has the segment_restored event but no rows. Verifier accepts via the transitional rule. Operator retries; retry detection skips Step 1, re-runs Step 2.
  • Step 1 and Step 2 succeed, Step 3 fails: chain has event, rows present, segment row still points at original purge event. Verifier accepts via the transitional rule (segment_restored event id is greater than current live_purged_event_id). Operator retries; retry detection skips Steps 1 and 2, runs only Step 3.
  • Operator abandons mid-flight: chain has segment_restored event but segment row never UPDATEd. Verifier accepts indefinitely. Operationally weird (segment in transitional state forever) but no integrity issue.

Retry detection in restore()

On entry to restore(), before doing any work:

$existing_restored_event = lookup segment_restored event referencing this segment
                          with id > segment.live_purged_event_id;

if ($existing_restored_event exists) {
  // Step 1 already happened.
  if (rows fully present in [from_id, to_id]) {
    // Steps 1 and 2 done; finalize Step 3.
    goto step_3 with $existing_restored_event;
  } else {
    // Step 2 partial or never ran. Re-run Steps 2 + 3.
    goto step_2 with $existing_restored_event;
  }
}
// No existing event - run Step 1.
goto step_1;

Concurrency

  • Concurrent logger writes during Step 2: proceed unblocked, land at chain head. Restored rows go to OLD ids in a disjoint window; no row-id conflict. Chain integrity preserved via UNIQUE(chain, previous_hash) and per-row HMAC.
  • Concurrent restore of the same segment: both reach Step 1, both emit segment_restored events under serialized lock. Now two segment_restored events exist for the same segment. Verifier rule: latest event id wins for supersession; older event is benign (segment row's live_purged_event_id will be set to the latest at Step 3). At Step 2 both transactions try the same row INSERTs; second hits PRIMARY KEY conflict, rolls back. First continues to Step 3. Acceptable but produces a benign extra chain event -- operators may want a "Step 0 check" rejecting restore if an in-flight event already exists.
  • Concurrent archive on the same chain different segment: archive's Phase B chain-event INSERT serializes with restore's Step 1 via the same chain-write lock. Both succeed sequentially.

Comparison with the alternatives considered

  • Schema marker approach (originally proposed in this issue): per-segment restore_in_progress column. Cleaner from an operator-UI standpoint (admin can show "in progress" segments) but requires a schema migration, update hook, and verifier rule. This approach (Approach D) gets the same operational benefit without the schema cost.
  • Advisory locks (MySQL GET_LOCK / PostgreSQL pg_advisory_lock): keep restore's structure unchanged and swap the chain-write lock backend to use connection-scoped advisory locks instead of the semaphore table. Smaller code change but introduces a custom lock backend running in parallel with Drupal's standard one. Doesn't leverage the chain event mechanism that already exists.
  • Drop restore-as-re-insertion entirely: make archives read-only; operators view archive content without re-INSERTing into audit_trail. Larger product-level decision; out of scope for this technical issue.

This proposal uses the existing chain-event mechanism as the in-progress marker. No schema change, no new lock backend, no new product model.

Related

  • #3591896 (parent: narrowed archiveSegment lock scope but deferred restore for this design work).
  • #3591878 (grandparent: introduced the TTL-renewal workaround that this issue's resolution replaces for restore).

Remaining tasks

  • Refactor restore() and restoreLocked() into Step 1 / Step 2 / Step 3 with the new ordering (event first).
  • Add retry-detection logic in restore() entry: detect existing segment_restored event and partial row presence; resume from the appropriate step.
  • Verifier extension for the transitional state: accept rows present in [from_id, to_id] and stale segment.live_purged_event_id when a segment_restored event with greater id exists.
  • Document the transitional state in docs/architecture.md and docs/verification.md.
  • Delete renewChainWriteLockIfStale(), CHAIN_WRITE_LOCK_TTL_S, CHAIN_WRITE_LOCK_RENEW_INTERVAL_S -- no longer needed once restore narrows.
  • Kernel tests: happy-path restore; Step 1 succeeds + Step 2 fails (retry resumes); Steps 1-2 succeed + Step 3 fails (retry finalizes); concurrent logger write during Step 2 (verifier accepts); concurrent same-segment restore (PK conflict handled, two benign events).

API changes

None for external callers.

Data model changes

None.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mably created an issue. See original summary.

mably’s picture

Category: Feature request » Task
mably’s picture

Status: Active » Postponed
mably’s picture

Title: Narrow restoreLocked() chain-write lock scope (needs per-segment restore_in_progress state) » Narrow restoreLocked() chain-write lock scope: emit segment_restored event first, replay rows without the lock
Issue summary: View changes

mably’s picture

Status: Postponed » Needs review

  • mably committed b53d8022 on 1.x
    task: #3591904 Narrow restoreLocked() chain-write lock scope: emit...
mably’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.