Skip to content

fix(restorer): return ErrWALNotFound on archive-get not-found exit#106

Open
chobostar wants to merge 1 commit into
operasoftware:mainfrom
chobostar:fix/wal-restore-errwalnotfound
Open

fix(restorer): return ErrWALNotFound on archive-get not-found exit#106
chobostar wants to merge 1 commit into
operasoftware:mainfrom
chobostar:fix/wal-restore-errwalnotfound

Conversation

@chobostar

@chobostar chobostar commented Jul 23, 2026

Copy link
Copy Markdown

What

WALRestorer.Restore now returns the ErrWALNotFound sentinel when pgbackrest archive-get reports the WAL segment as missing (exit code 1), instead of collapsing
every non-zero exit code into a single generic error.

#105

Why

ErrWALNotFound is defined in internal/pgbackrest/restorer/wal.go and checked in three
places to tell the normal end-of-WAL case apart from a real failure:

  • internal/pgbackrest/restorer/wal.go — Info vs Warning log level for the requested WAL
  • internal/cnpgi/common/wal.go — return the codes.NotFound gRPC status
    (newWALNotFoundError) instead of a raw error
  • internal/cnpgi/common/wal.goisEndOfWALStream() prefetch/end-of-stream detection

But nothing ever produced it: the only place that runs archive-get returned a generic
fmt.Errorf for any non-zero exit code, so errors.Is(err, ErrWALNotFound) was always
false and all three checks were dead code.

This does not change whether recovery works — PostgreSQL treats any non-zero restore_command
as "WAL unavailable" and switches to streaming either way. The fix just stops the plugin from
misreporting the normal end-of-WAL:

  • Logs (always). A replica tries the archive for each new WAL before it has been archived;
    this normal miss was logged as Warning ("Failed restoring WAL file") and now logs as
    Info ("WAL file not found"). Healthy replicas stop emitting warnings that look like
    failures.
  • Fewer object-store lookups (only when wal.maxParallel > 1). With parallel prefetch the
    plugin can now set its end-of-wal-stream flag and stop re-requesting segments it already
    knows are missing. No effect at the default maxParallel=1.
  • Diagnostics. exit 103 (RepoInvalidError — stanza/repo not initialized) is no longer
    indistinguishable from a normal "WAL not there yet".

pgbackrest archive-get returns exit code 1 specifically when a segment is not present
in an otherwise-valid repository (its default int result = 1; pgBackRest exception codes
start at 25, so 1 is unambiguously "not found"). Mapping that code restores the
intended behaviour and keeps every other non-zero code (including 103) as a real error.

Behaviour

archive-get exit Before After
1 (WAL not found) generic error ErrWALNotFound
103 (repo invalid) generic error generic error (unchanged)
other non-zero generic error generic error (unchanged)
non-exit failure wrapped error wrapped error (unchanged)

Recovery correctness is unchanged — PostgreSQL already treats any non-zero
restore_command as "WAL unavailable" and switches to streaming; this only makes the
plugin signal and log the two conditions correctly.

Changes

  • internal/pgbackrest/restorer/wal.go: extract exit-code handling into walRestoreError
    and map the not-found exit code to ErrWALNotFound.
  • internal/pgbackrest/restorer/wal_test.go: unit tests for walRestoreError covering the
    not-found code, other non-zero codes, and non-exit failures.

Testing

go build ./...                                     # ok
gofmt -l internal/pgbackrest/restorer/...          # clean
go vet ./internal/pgbackrest/restorer/...          # clean
golangci-lint run ./internal/pgbackrest/restorer/... # 0 issues (v2.1.5)
go test ./internal/pgbackrest/restorer/...         # ok — 6 specs, 3 new

Checklist

  • Conventional commit with Signed-off-by (DCO)
  • Unit tests added
  • gofmt / go vet / golangci-lint (v2.1.5) clean
  • No behaviour change to recovery correctness; scope limited to error typing

Pgbackrest archive-get exits with code 1 when a WAL segment is absent from a valid
repository, which is the normal end-of-WAL signal during recovery. The restore path
collapsed every non-zero exit into a single generic error, so ErrWALNotFound was never
returned and its three call sites were dead code: exit 1 (not found) was
indistinguishable from exit 103 (repository invalid). Map the not-found exit code to
ErrWALNotFound so end-of-WAL detection, the NotFound gRPC status and info-level logging
behave as intended.

Signed-off-by: chobostar <chobostar85@gmail.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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