diff --git a/internal/cnpgi/common/errors.go b/internal/cnpgi/common/errors.go index 72c827f..841137d 100644 --- a/internal/cnpgi/common/errors.go +++ b/internal/cnpgi/common/errors.go @@ -35,19 +35,18 @@ var ErrEndOfWALStreamReached = status.Errorf(codes.OutOfRange, "end of WAL reach var ErrMissingPermissions = status.Errorf(codes.FailedPrecondition, "no permission to download the backup credentials, retrying") -// newWALNotFoundError returns an error indicating that the -// requested WAL file is not present in the object store. -// It carries a gRPC NotFound status so the operator can -// treat it as a terminal condition. +// newWALNotFoundError reports that the requested WAL file is not +// present in the object store. Emits codes.NotFound: this is a +// terminal condition (the file won't appear on retry). func newWALNotFoundError(walName string) error { return status.Errorf(codes.NotFound, "wal %q not found", walName) } -// newUnavailableError returns an error indicating that -// downloading the given WAL file failed for a transient -// reason (e.g. a connectivity blip). It carries a gRPC -// Unavailable status so the operator will retry the -// request. +// newUnavailableError reports that downloading the WAL file failed +// for a reason expected to be transient (a barman-cloud connectivity +// blip, or a generic exit code that in practice covers retryable +// conditions too). Emits codes.Unavailable: per gRPC conventions, +// the canonical signal for "retry may succeed". func newUnavailableError(walName string, err error) error { return status.Errorf( codes.Unavailable, @@ -57,10 +56,9 @@ func newUnavailableError(walName string, err error) error { ) } -// newInvalidWALNameError returns an error indicating that -// the requested WAL name is not valid. It carries a gRPC -// InvalidArgument status so the operator treats it as a -// terminal condition. +// newInvalidWALNameError reports that the requested WAL name is +// not a valid name. Emits codes.InvalidArgument: this is a +// terminal condition (the same name won't become valid on retry). func newInvalidWALNameError(walName string, err error) error { return status.Errorf( codes.InvalidArgument, @@ -70,10 +68,10 @@ func newInvalidWALNameError(walName string, err error) error { ) } -// newInternalWALRestoreError returns an error indicating -// that downloading the given WAL file failed for an -// unclassified reason. It carries a gRPC Internal status -// so the operator treats it as a terminal condition. +// newInternalWALRestoreError reports that downloading the WAL +// file failed for an unclassified reason. Emits codes.Internal: +// we have no positive signal that retry would help, so by gRPC +// convention this is treated as terminal. func newInternalWALRestoreError(walName string, err error) error { return status.Errorf( codes.Internal, diff --git a/internal/cnpgi/common/wal.go b/internal/cnpgi/common/wal.go index b18f7fd..489f71b 100644 --- a/internal/cnpgi/common/wal.go +++ b/internal/cnpgi/common/wal.go @@ -369,22 +369,23 @@ func (w WALServiceImplementation) restoreFromBarmanObjectStore( walErr := walStatus[0].Err switch { case errors.Is(walErr, barmanRestorer.ErrWALNotFound): - // A missing WAL is a terminal condition: surface it as a - // gRPC NotFound so the operator stops retrying. return newWALNotFoundError(walName) case errors.Is(walErr, barmanRestorer.ErrInvalidWalName): - // A malformed WAL name will never succeed on retry. + // A malformed WAL name will never become valid on retry. return newInvalidWALNameError(walName, walErr) case errors.Is(walErr, barmanRestorer.ErrConnectivity), errors.Is(walErr, barmanRestorer.ErrGeneric): - // Connectivity and generic barman-cloud failures are - // considered transient: surface them as gRPC Unavailable - // so the operator retries the download. + // barman-cloud exit codes 2 (connectivity) and 4 + // (generic) both surface conditions that are retryable + // in practice — barman uses the "generic" bucket for + // some connection-class failures too, not just exit 2. + // Emit codes.Unavailable so the caller retries. return newUnavailableError(walName, walErr) default: - // Unrecognized exit codes and unexpected failures (e.g. the - // barman-cloud command could not be executed) are surfaced - // as gRPC Internal so the operator treats them as terminal. + // Unrecognized exit codes and unexpected failures + // (e.g. the barman-cloud command could not be + // executed). No positive signal that retry would + // help; emit codes.Internal. return newInternalWALRestoreError(walName, walErr) } }