Conversation
GabriFedi97
force-pushed
the
dev/248
branch
3 times, most recently
from
September 17, 2026 07:15
ed45d49 to
dbfa6e2
Compare
A backup taken on a standby runs `klio backup run --wait-for-wals=false`, because a standby cannot force a WAL switch on the primary and its last segment may take a long time to arrive. The WAL server, however, returned from CloseBackup without enqueueing the post-backup task whenever a WAL file was still missing, relying on the client to call again. A client that does not wait never calls again, so the backup completed with no error while its tier2 relay, consumer-side verification and maintenance silently never ran. CloseBackupRequest gains a `wait_for_wals` flag carrying the client's intent. The server defers the task only when the client will retry; otherwise it enqueues the task right away and still reports the missing WAL files, which the client now logs when it does not wait for them. The tier2 recovery e2e now uses a two-instance source cluster so the backup, with the default prefer-standby target, is taken on the standby, and switches WAL on the primary after the backup so the recovery can reach the backup end. Closes #248 Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Rename the new CloseBackupRequest flag to `enqueue_without_wals` and invert it, so that a request without the flag behaves as before: the post-backup task is deferred until no WAL file is missing. Only a client that will not call again, i.e. a backup taken on a standby, opts in. The Server image is chosen in the Server resource while the sidecar image comes from the operator, so the two can be on different versions. With the previous polarity an old sidecar talking to a new server would have enqueued one task per retry while waiting for WALs on a primary. With this one neither skew direction changes the primary path. Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
The flag documents a client that does not wait for the last WAL to be archived, whatever the reason. Reword the proto comment, the server-side comment in CloseBackup and the two test comments accordingly, so a standby backup reads as one user of the flag rather than its definition. Signed-off-by: Gabriele Fedi <gabriele.fedi@enterprisedb.com>
GabriFedi97
force-pushed
the
dev/248
branch
from
September 17, 2026 07:41
dbfa6e2 to
0844b69
Compare
GabriFedi97
approved these changes
Sep 17, 2026
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
2 participants
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.
When a backup is taken on a standby, the sidecar runs
klio backup runwith--wait-for-wals=false, since a standby can't force a WAL switch on the primary and the last segment might take a while to show up. The problem is thatCloseBackupon the WAL server was returning early whenever a WAL file was still missing, without enqueueing the post-backup task, on the assumption that the client would call again. A client that isn't waiting never calls again. So the CNPG Backup went green, nothing was logged, and the tier2 relay (plus the consumer-side verification and maintenance) simply never happened.On a primary this never showed up because
pg_backup_stopswitches WAL, so the missing segment arrives within seconds and the retry loop picks it up. That's also why the tier2 e2e tests never caught it: the source clusters had a single instance.The fix adds an
enqueue_without_walsflag toCloseBackupRequest. A client that won't retry sets it and the server enqueues the task right away, still reporting the missing WALs (the client logs them now). Without the flag the behaviour is unchanged, so an old sidecar against a new server, or the other way around, keeps working exactly as today on the primary path.Testing:
CloseBackupwith an embedded NATS server, covering both the standby and the primary caseRecoverClusterFromTier2now uses a two-instance source cluster, so with the defaultprefer-standbytarget the backup is taken on the standby. It fails on main (tier2 replication never completes) and passes with this change. It also switches WAL on the primary after the backup, otherwise on an idle cluster the recovery would wait forever for the segment holding the backup end.Closes #248
Signed-off-by: Armando Ruocco armando.ruocco@enterprisedb.com