# PiTR Serialization Constraint Design

**Date:** 2026-05-18  
**Status:** Approved

## Problem

Compaction-accelerated PiTR restores trigger a `CompactBackup` job that writes compacted SST files back to the source S3 log backup bucket. If two PiTR restores share the same log backup and both reach the compaction step concurrently, each spawns its own `CompactBackup` job writing to the same path prefix. The resulting duplicate SST files are later ingested twice, exhausting disk space on the restored cluster and causing the restore to fail irrecoverably.

Snapshot restores are **not** affected: they do not perform compaction and do not share S3 write paths.

## Design Goals

1. Prevent two or more PiTR restores from running concurrently against the **same log backup**.
2. Provide an operator escape hatch (`AllowConcurrentPiTR`) for clusters where the constraint must be bypassed.
3. Keep changes surgical: no modifications to `restore_base.go` or the snapshot restore path.
4. Remain safe under TOCTOU races: a best-effort guard at creation time is acceptable, but the compaction gate must be atomic.

## Non-Goals

- Serializing snapshot restores.
- Serializing PiTR restores across **different** log backups.
- Queuing concurrent PiTR requests (failed = user retries).

## Two-Layer Defense

### Layer 1 — Best-Effort Guard at Create Time

**Location:** `PiTRestore.makeCreateRestoreEntity()` in `app/models/restore/impl/pitr_restore.go`

**When:** Called during the `createRestoreEntity()` flow, before the entity is inserted into the database.

**Logic:**

```
if AllowConcurrentPiTR not set for this cluster:
    count = restoreRepo.Count(
        LogBackupIDEq(logBackup.ID),
        ModeEq(PiTR),
        StatusIn(PENDING, RUNNING),
    )
    if count > 0:
        return bizerr_wrapper.NewConflictErr(ErrMsgPiTRConcurrentRestoreNotAllowed)
```

**Scope:** Applies to **all** PiTR restores, including those where compaction will later be skipped. This is intentionally conservative: it is simpler to enforce at the API boundary than to predict at create time whether compaction will be needed.

**Config lookup limitation:** Provider/region are omitted (`"", ""`) because the target cluster may not exist yet at create time (restore-to-new path). Override entries that gate on provider or region won't match at Layer 1 but will still apply at Layer 2.

**Race window:** Two simultaneous requests can both see `count == 0` and both proceed to create an entity. This is acceptable — Layer 2 catches the second one before any compaction starts.

**Error code:** HTTP 409 (`ErrCodeRecordExist`) / `ErrMsgPiTRConcurrentRestoreNotAllowed`

**Config lookup:** Uses `config.Global().BackupConfig.RestoreOverrides.GetMatched(logBackup.TenantID, logBackup.ProjectID, logBackup.ClusterID, "", "")`. Empty provider/region is intentional — no extra RPC needed and `GetMatched` falls back correctly.

### Layer 2 — Hard Gate in Compaction Action

**Location:** `RunCompactBackupForRestore.execute()` in `app/workflow/action/restore/pitr/create/run_compact_backup.go`

**When:** Runs inside the existing `transProvider.Transaction(...)` block, immediately before `compactBackupModel.AsyncCreate()`.

**Logic:**

```
inside transaction:
    count = restoreRepo.Count(
        LogBackupIDEq(restore.LogBackupID),
        ModeEq(PiTR),
        StatusIn(RUNNING),
        IDNotIn(restore.ID),      // exclude self
    )
    if count > 0:
        return fmt.Errorf("PiTR restore %d for log backup %d already running; "+
                           "concurrent PiTR not allowed", restore.ID, restore.LogBackupID)
```

**Why this reduces — but does not eliminate — concurrent compaction:** Two PiTR restores that both pass Layer 1 (TOCTOU race) will each execute `execute()`. The count query here runs before `transProvider.Transaction()`, so it is not truly atomic with the `AsyncCreate` call. In practice this window is very small: both restores would need to reach `execute()` simultaneously, each see count=0 (IDNotIn excludes self), and then both spawn compaction. Layer 2 significantly reduces this probability but does not provide a strict serialization guarantee. A future improvement could perform the count inside the DB transaction for a stronger guarantee.

**What happens on failure:** The workflow action returns an error → the DAG fails → the restore entity is transitioned to `FAILED` status by the workflow engine's error handler. The user must create a new restore request.

**Gating condition:** This check only runs when `needCompaction()` returns true — i.e., when `CompactionAcceleratedPiTR` or CRRF is active. Non-compaction PiTR restores skip Layer 2 entirely. Note: Layer 1 applies to **all** PiTR restores (intentionally conservative; see Layer 1 section).

**Override:** `override.AllowConcurrentPiTR` bypasses this check (same override struct used in Layer 1).

## Config Change

### `internal/config/config.go` — `RestoreOverride` struct

Add one boolean field:

```go
// AllowConcurrentPiTR bypasses the PiTR serialization constraint.
// When false (default), at most one PiTR restore per log backup may be in PENDING/RUNNING state.
AllowConcurrentPiTR bool `mapstructure:"allowConcurrentPiTR" default:"false"`
```

## Error Code

### `app/config/errcode.go`

```go
ErrMsgPiTRConcurrentRestoreNotAllowed = "a PiTR restore for the same log backup is already in progress; " +
    "please wait for it to finish before creating a new one"
```

## Affected Query Filters

All filters are already present in `app/repo/entity/autogenerated_entity.go` — no new repo methods needed:

| Filter | Line |
|--------|------|
| `RestoreQuerySet.LogBackupIDEq()` | 33271 |
| `RestoreQuerySet.ModeEq()` | 33339 |
| `RestoreQuerySet.StatusIn()` | 34121 |
| `RestoreQuerySet.IDNotIn()` | 33211 (Layer 2 only) |

## Files Changed

| File | Change |
|------|--------|
| `internal/config/config.go` | Add `AllowConcurrentPiTR bool` to `RestoreOverride` |
| `app/config/errcode.go` | Add `ErrMsgPiTRConcurrentRestoreNotAllowed` constant |
| `app/models/restore/impl/pitr_restore.go` | Layer 1 check in `makeCreateRestoreEntity()` |
| `app/workflow/action/restore/pitr/create/run_compact_backup.go` | Layer 2 check in `execute()` |
| `app/models/restore/impl/pitr_restore_test.go` (new or existing) | Unit tests for Layer 1 |
| `app/workflow/action/restore/pitr/create/run_compact_backup_test.go` (new or existing) | Unit tests for Layer 2 |

**No changes to:** `restore_base.go`, snapshot restore paths, repo layer, IDL/gRPC layer.

## Test Plan

### Layer 1 Unit Tests (`makeCreateRestoreEntity`)

| Case | Expected |
|------|----------|
| No active PiTR for same log backup | Entity created, no error |
| 1 RUNNING PiTR for same log backup, no override | `ErrMsgPiTRConcurrentRestoreNotAllowed` (bizerr) |
| 1 PENDING PiTR for same log backup, no override | `ErrMsgPiTRConcurrentRestoreNotAllowed` (bizerr) |
| 1 RUNNING PiTR, `AllowConcurrentPiTR = true` | Entity created, no error |
| `logBackup == nil` (snapshot path) | Check skipped, entity created |

### Layer 2 Unit Tests (`execute`)

| Case | Expected |
|------|----------|
| No other RUNNING PiTR for same log backup | `AsyncCreate` called, success |
| 1 RUNNING PiTR for same log backup (≠ self), no override | `execute` returns error; `AsyncCreate` NOT called |
| Self is the only RUNNING PiTR | Check passes (IDNotIn excludes self) |
| `AllowConcurrentPiTR = true` | Check skipped, `AsyncCreate` called |

## Risk Assessment

- **No schema changes** — purely application-level guard.
- **No new repo methods** — uses existing query set filters.
- **Backward compatible** — new field defaults to `false`; existing configs unchanged.
- **Layer 2 race safety** — atomic with `AsyncCreate` inside a transaction.
- **Escape hatch available** — `AllowConcurrentPiTR` per-cluster override for ops.