fix(restorer): return ErrWALNotFound on archive-get not-found exit#106
Open
chobostar wants to merge 1 commit into
Open
fix(restorer): return ErrWALNotFound on archive-get not-found exit#106chobostar wants to merge 1 commit into
chobostar wants to merge 1 commit into
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
WALRestorer.Restorenow returns theErrWALNotFoundsentinel whenpgbackrest archive-getreports the WAL segment as missing (exit code1), instead of collapsingevery non-zero exit code into a single generic error.
#105
Why
ErrWALNotFoundis defined ininternal/pgbackrest/restorer/wal.goand checked in threeplaces 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 WALinternal/cnpgi/common/wal.go— return thecodes.NotFoundgRPC status(
newWALNotFoundError) instead of a raw errorinternal/cnpgi/common/wal.go—isEndOfWALStream()prefetch/end-of-stream detectionBut nothing ever produced it: the only place that runs
archive-getreturned a genericfmt.Errorffor any non-zero exit code, soerrors.Is(err, ErrWALNotFound)was alwaysfalseand all three checks were dead code.This does not change whether recovery works — PostgreSQL treats any non-zero
restore_commandas "WAL unavailable" and switches to streaming either way. The fix just stops the plugin from
misreporting the normal end-of-WAL:
this normal miss was logged as
Warning("Failed restoring WAL file") and now logs asInfo("WAL file not found"). Healthy replicas stop emitting warnings that look likefailures.
wal.maxParallel > 1). With parallel prefetch theplugin can now set its
end-of-wal-streamflag and stop re-requesting segments it alreadyknows are missing. No effect at the default
maxParallel=1.103(RepoInvalidError— stanza/repo not initialized) is no longerindistinguishable from a normal "WAL not there yet".
pgbackrest archive-getreturns exit code1specifically when a segment is not presentin an otherwise-valid repository (its default
int result = 1; pgBackRest exception codesstart at
25, so1is unambiguously "not found"). Mapping that code restores theintended behaviour and keeps every other non-zero code (including
103) as a real error.Behaviour
archive-getexit1(WAL not found)ErrWALNotFound103(repo invalid)Recovery correctness is unchanged — PostgreSQL already treats any non-zero
restore_commandas "WAL unavailable" and switches to streaming; this only makes theplugin signal and log the two conditions correctly.
Changes
internal/pgbackrest/restorer/wal.go: extract exit-code handling intowalRestoreErrorand map the not-found exit code to
ErrWALNotFound.internal/pgbackrest/restorer/wal_test.go: unit tests forwalRestoreErrorcovering thenot-found code, other non-zero codes, and non-exit failures.
Testing
Checklist
Signed-off-by(DCO)gofmt/go vet/golangci-lint(v2.1.5) clean