From f0c47a7bd69abd6aba7eb2b40bba30f0444d57f3 Mon Sep 17 00:00:00 2001 From: Marco Nenciarini Date: Fri, 17 Jul 2026 11:56:23 +0200 Subject: [PATCH] fix: clear stale end-of-wal-stream flag before pg_rewind restore The previous commit stopped pg_rewind restores from checking the end-of-wal-stream flag, but left an existing flag on disk untouched. A flag set by a normal-recovery invocation before this pod was demoted would then resurface once the rewind finished and wrongly abort the following normal-recovery invocation. Clear the flag unconditionally in rewind mode, before the spool short-circuit, so a request for a WAL file already staged in the spool cannot skip the clear. Signed-off-by: Marco Nenciarini --- internal/cnpgi/common/wal.go | 30 ++++++++++++++++++++++++++ internal/cnpgi/common/wal_test.go | 36 +++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/internal/cnpgi/common/wal.go b/internal/cnpgi/common/wal.go index 8a196cc..1d4fd30 100644 --- a/internal/cnpgi/common/wal.go +++ b/internal/cnpgi/common/wal.go @@ -323,6 +323,20 @@ func (w WALServiceImplementation) restoreFromBarmanObjectStore( return fmt.Errorf("while creating the restorer: %w", err) } + // A flag left over from a normal-recovery invocation that ran before this pod + // was demoted must not survive into a pg_rewind restore: the flag machinery + // does not apply while restoring on behalf of pg_rewind (see + // shouldUseEndOfWALStreamFlag below), so it is never checked here, but left + // untouched it would resurface and wrongly abort the first normal-recovery + // invocation that runs once the rewind is done. This runs unconditionally, + // before the spool short-circuit in Step 1, so a request for a WAL file that + // happens to already be staged in the spool cannot skip the clear. + if rewindMode { + if err := clearEndOfWALStreamFlag(walRestorer); err != nil { + return err + } + } + // Step 1: check if this WAL file is not already in the spool var wasInSpool bool if wasInSpool, err = walRestorer.RestoreFromSpool(walName, destinationPath); err != nil { @@ -526,6 +540,22 @@ func checkEndOfWALStreamFlag(walRestorer *barmanRestorer.WALRestorer) error { return nil } +// clearEndOfWALStreamFlag removes the end-of-wal-stream flag, if present, without +// treating it as an error. It is used instead of checkEndOfWALStreamFlag when +// restoring on behalf of pg_rewind, which must not abort on a flag it did not +// set itself. +func clearEndOfWALStreamFlag(walRestorer *barmanRestorer.WALRestorer) error { + contain, err := walRestorer.IsEndOfWALStream() + if err != nil { + return err + } + + if contain { + return walRestorer.ResetEndOfWalStream() + } + return nil +} + // isEndOfWALStream returns true if one of the downloads has returned // a file-not-found error. func isEndOfWALStream(results []barmanRestorer.Result) bool { diff --git a/internal/cnpgi/common/wal_test.go b/internal/cnpgi/common/wal_test.go index e5306d7..1529800 100644 --- a/internal/cnpgi/common/wal_test.go +++ b/internal/cnpgi/common/wal_test.go @@ -20,7 +20,10 @@ SPDX-License-Identifier: Apache-2.0 package common import ( + "context" + barmanapi "github.com/cloudnative-pg/barman-cloud/pkg/api" + barmanRestorer "github.com/cloudnative-pg/barman-cloud/pkg/restorer" cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -141,3 +144,36 @@ var _ = Describe("shouldUseEndOfWALStreamFlag", func() { Entry("rewind mode wins over streaming availability", clusterWithPrimary("cluster-1"), "cluster-2", true, false), ) }) + +var _ = Describe("clearEndOfWALStreamFlag", func() { + newRestorer := func() *barmanRestorer.WALRestorer { + restorer, err := barmanRestorer.New(context.Background(), nil, GinkgoT().TempDir()) + Expect(err).ToNot(HaveOccurred()) + return restorer + } + + It("is a no-op when the flag is not set", func() { + restorer := newRestorer() + + Expect(clearEndOfWALStreamFlag(restorer)).To(Succeed()) + + isEOS, err := restorer.IsEndOfWALStream() + Expect(err).ToNot(HaveOccurred()) + Expect(isEOS).To(BeFalse()) + }) + + // Regression guard: a flag left over from a normal-recovery invocation that + // ran before this pod was demoted must not survive a pg_rewind restore, or + // it would resurface and wrongly abort the first normal-recovery invocation + // that runs once the rewind is done. + It("removes a pre-existing flag without returning an error", func() { + restorer := newRestorer() + Expect(restorer.SetEndOfWALStream()).To(Succeed()) + + Expect(clearEndOfWALStreamFlag(restorer)).To(Succeed()) + + isEOS, err := restorer.IsEndOfWALStream() + Expect(err).ToNot(HaveOccurred()) + Expect(isEOS).To(BeFalse()) + }) +})