From 4e942a84740f9c26c9e5101c8b0cf4fe96b26927 Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Tue, 15 Sep 2026 15:49:12 +0200 Subject: [PATCH 1/3] fix: relay standby backups to tier2 without waiting for the last WAL 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 --- core/cmd/backup/run.go | 18 ++- core/internal/grpc/klio_wal.pb.go | 22 ++- core/internal/server/walserver/backup.go | 9 +- core/internal/server/walserver/backup_test.go | 153 ++++++++++++++++++ core/proto/klio_wal.proto | 7 + documentation/web/docs/developer/_protocol.md | 1 + .../web/docs/developer/running-e2e-tests.md | 5 +- operator/test/e2e/tier2_recovery_test.go | 37 +++-- 8 files changed, 230 insertions(+), 22 deletions(-) create mode 100644 core/internal/server/walserver/backup_test.go diff --git a/core/cmd/backup/run.go b/core/cmd/backup/run.go index 0b06522f..17919c8b 100644 --- a/core/cmd/backup/run.go +++ b/core/cmd/backup/run.go @@ -158,6 +158,7 @@ func runBackup(cmd *cobra.Command, _ []string) error { EndWal: metadata.EndWAL, SegmentSize: metadata.SegmentSize, SendToTier2: tier2, + WaitForWals: waitWALs, Tier2RetentionPolicy: marshalTier2RetentionPolicy(cmd.Context(), &configuration), Tier2CompressionPolicy: marshalTier2CompressionPolicy(cmd.Context(), &configuration), }) @@ -167,14 +168,21 @@ func runBackup(cmd *cobra.Command, _ []string) error { backupfailure.RepositoryError.ExitCode) } - if waitWALs && len(result.GetMissingWalFiles()) > 0 { + if len(result.GetMissingWalFiles()) > 0 { + if waitWALs { + contextLogger.Info( + "Detected missing WAL files, waiting for 5 seconds", + "missingWALFiles", result.GetMissingWalFiles(), + ) + time.Sleep(5 * time.Second) + + continue + } + contextLogger.Info( - "Detected missing WAL files, waiting for 5 seconds", + "Detected missing WAL files, not waiting for them", "missingWALFiles", result.GetMissingWalFiles(), ) - time.Sleep(5 * time.Second) - - continue } if result.GetTier2Schedule() { diff --git a/core/internal/grpc/klio_wal.pb.go b/core/internal/grpc/klio_wal.pb.go index 9cc51d6d..158c03b6 100644 --- a/core/internal/grpc/klio_wal.pb.go +++ b/core/internal/grpc/klio_wal.pb.go @@ -730,8 +730,14 @@ type CloseBackupRequest struct { Tier2RetentionPolicy string `protobuf:"bytes,9,opt,name=tier2_retention_policy,json=tier2RetentionPolicy,proto3" json:"tier2_retention_policy,omitempty"` // When present, set the tier2 compression policy to the specified JSON-serialized policy. Tier2CompressionPolicy string `protobuf:"bytes,10,opt,name=tier2_compression_policy,json=tier2CompressionPolicy,proto3" json:"tier2_compression_policy,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // True when the client will call CloseBackup again until no WAL file is + // missing. The server then defers the post-backup task until that point. + // When false the task is enqueued right away, even if WAL files are still + // missing: a backup taken on a standby cannot force a WAL switch on the + // primary, so its last segment may take a long time to arrive. + WaitForWals bool `protobuf:"varint,11,opt,name=wait_for_wals,json=waitForWals,proto3" json:"wait_for_wals,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *CloseBackupRequest) Reset() { @@ -827,6 +833,13 @@ func (x *CloseBackupRequest) GetTier2CompressionPolicy() string { return "" } +func (x *CloseBackupRequest) GetWaitForWals() bool { + if x != nil { + return x.WaitForWals + } + return false +} + // This is sent by the WAL server in response to a CloseBackupRequest // message. type CloseBackupResult struct { @@ -930,7 +943,7 @@ const file_proto_klio_wal_proto_rawDesc = "" + "\fStartWALFile\x12!\n" + "\fklio_version\x18\x01 \x01(\x04R\vklioVersion\x12\x1f\n" + "\vfile_length\x18\x02 \x01(\x04R\n" + - "fileLength\"\xe1\x02\n" + + "fileLength\"\x85\x03\n" + "\x12CloseBackupRequest\x12!\n" + "\fcluster_name\x18\x01 \x01(\tR\vclusterName\x12\x1f\n" + "\vbackup_name\x18\x03 \x01(\tR\n" + @@ -942,7 +955,8 @@ const file_proto_klio_wal_proto_rawDesc = "" + "\rsend_to_tier2\x18\b \x01(\bR\vsendToTier2\x124\n" + "\x16tier2_retention_policy\x18\t \x01(\tR\x14tier2RetentionPolicy\x128\n" + "\x18tier2_compression_policy\x18\n" + - " \x01(\tR\x16tier2CompressionPolicy\"f\n" + + " \x01(\tR\x16tier2CompressionPolicy\x12\"\n" + + "\rwait_for_wals\x18\v \x01(\bR\vwaitForWals\"f\n" + "\x11CloseBackupResult\x12%\n" + "\x0etier2_schedule\x18\x01 \x01(\bR\rtier2Schedule\x12*\n" + "\x11missing_wal_files\x18\x02 \x03(\tR\x0fmissingWalFiles2\xd8\x03\n" + diff --git a/core/internal/server/walserver/backup.go b/core/internal/server/walserver/backup.go index fefe4d85..00913920 100644 --- a/core/internal/server/walserver/backup.go +++ b/core/internal/server/walserver/backup.go @@ -45,7 +45,12 @@ func (w *Implementation) CloseBackup( return nil, err } - if len(missingWALFiles) > 0 { + // A client that waits for the WALs calls CloseBackup again until none is + // missing, so the task is deferred to that call. A client that does not + // wait (a backup taken on a standby, which cannot force a WAL switch on + // the primary) calls only once: the task must be enqueued now or the + // backup is never processed. + if len(missingWALFiles) > 0 && request.GetWaitForWals() { return &grpc.CloseBackupResult{ Tier2Schedule: false, MissingWalFiles: missingWALFiles, @@ -67,7 +72,7 @@ func (w *Implementation) CloseBackup( return &grpc.CloseBackupResult{ Tier2Schedule: w.queue != nil && request.GetSendToTier2(), - MissingWalFiles: nil, + MissingWalFiles: missingWALFiles, }, nil } diff --git a/core/internal/server/walserver/backup_test.go b/core/internal/server/walserver/backup_test.go new file mode 100644 index 00000000..cb1b27f9 --- /dev/null +++ b/core/internal/server/walserver/backup_test.go @@ -0,0 +1,153 @@ +/* +Copyright © contributors to CloudNativePG, established as +CloudNativePG a Series of LF Projects, LLC. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package walserver + +import ( + "context" + "path" + "testing" + "time" + + "github.com/nats-io/nats-server/v2/server" + "github.com/nats-io/nats.go" + "github.com/spf13/afero" + "github.com/stretchr/testify/require" + + "github.com/cloudnative-pg/klio/core/internal/grpc" + "github.com/cloudnative-pg/klio/core/internal/queue" + "github.com/cloudnative-pg/klio/core/internal/repository" +) + +const ( + testClusterName = "cluster-example" + testSegmentSize = 16 * 1024 * 1024 + testStartWAL = "000000010000000000000001" + testEndWAL = "000000010000000000000002" +) + +// newCloseBackupServer builds a WAL server whose repository holds only the +// backup's first WAL segment, so the last one is always reported missing, +// and whose queue is backed by an embedded NATS server. It returns the +// server and the queue connection to consume the enqueued tasks from. +func newCloseBackupServer(t *testing.T) (*Implementation, *queue.Conn) { + t.Helper() + + ns, err := server.NewServer(&server.Options{ + Host: "127.0.0.1", + Port: -1, + JetStream: true, + StoreDir: t.TempDir(), + }) + require.NoError(t, err) + go ns.Start() + require.True(t, ns.ReadyForConnections(4*time.Second), "NATS server not ready") + t.Cleanup(ns.Shutdown) + + nc, err := nats.Connect(ns.ClientURL()) + require.NoError(t, err) + t.Cleanup(nc.Close) + + q, err := queue.New(context.Background(), nc) + require.NoError(t, err) + + fs := afero.NewMemMapFs() + repoOpts := repository.Options{FS: fs, Password: "test-password"} + require.NoError(t, repository.Initialize(repoOpts)) + conn, err := repository.Open(repoOpts) + require.NoError(t, err) + t.Cleanup(conn.Close) + + // Only the first segment has been archived. + walPath := path.Join(testClusterName, testStartWAL[:16], testStartWAL) + require.NoError(t, afero.WriteFile(fs, walPath, []byte("wal"), 0o600)) + + return New(Options{Connection: conn, Queue: q}), q +} + +// receiveBackupTask consumes one backup task from the queue, or returns nil +// when none arrives within the timeout. +func receiveBackupTask(t *testing.T, q *queue.Conn, timeout time.Duration) *queue.BackupTask { + t.Helper() + + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + + received := make(chan *queue.BackupTask, 1) + go func() { + _ = q.ConsumeBackupReceivedMessages(ctx, func(_ context.Context, task *queue.BackupTask) error { + received <- task + cancel() + + return nil + }) + }() + + select { + case task := <-received: + return task + case <-ctx.Done(): + return nil + } +} + +func newCloseBackupRequest(waitForWALs bool) *grpc.CloseBackupRequest { + return &grpc.CloseBackupRequest{ + ClusterName: testClusterName, + BackupName: "backup-1", + Timeline: 1, + StartWal: testStartWAL, + EndWal: testEndWAL, + SegmentSize: testSegmentSize, + SendToTier2: true, + WaitForWals: waitForWALs, + } +} + +// TestCloseBackupMissingWALsWithoutWaitEnqueuesTask covers a backup taken on +// a standby: the client does not wait for the last WAL, so the post-backup +// task must be enqueued on this single call or the backup is never relayed. +func TestCloseBackupMissingWALsWithoutWaitEnqueuesTask(t *testing.T) { + impl, q := newCloseBackupServer(t) + + result, err := impl.CloseBackup(context.Background(), newCloseBackupRequest(false)) + require.NoError(t, err) + require.Equal(t, []string{testEndWAL}, result.GetMissingWalFiles()) + require.True(t, result.GetTier2Schedule()) + + task := receiveBackupTask(t, q, 5*time.Second) + require.NotNil(t, task, "the backup task must be enqueued even if WALs are missing") + require.Equal(t, testClusterName, task.ClusterName) + require.True(t, task.SendToTier2) +} + +// TestCloseBackupMissingWALsWithWaitDefersTask covers a backup taken on the +// primary: the client retries CloseBackup until no WAL is missing, so the +// task must not be enqueued before then. +func TestCloseBackupMissingWALsWithWaitDefersTask(t *testing.T) { + impl, q := newCloseBackupServer(t) + + result, err := impl.CloseBackup(context.Background(), newCloseBackupRequest(true)) + require.NoError(t, err) + require.Equal(t, []string{testEndWAL}, result.GetMissingWalFiles()) + require.False(t, result.GetTier2Schedule()) + + require.Nil(t, receiveBackupTask(t, q, time.Second), + "no backup task must be enqueued while the client is still waiting for WALs") +} diff --git a/core/proto/klio_wal.proto b/core/proto/klio_wal.proto index 2d936e0c..718bd5fa 100644 --- a/core/proto/klio_wal.proto +++ b/core/proto/klio_wal.proto @@ -164,6 +164,13 @@ message CloseBackupRequest { // When present, set the tier2 compression policy to the specified JSON-serialized policy. string tier2_compression_policy = 10; + + // True when the client will call CloseBackup again until no WAL file is + // missing. The server then defers the post-backup task until that point. + // When false the task is enqueued right away, even if WAL files are still + // missing: a backup taken on a standby cannot force a WAL switch on the + // primary, so its last segment may take a long time to arrive. + bool wait_for_wals = 11; } // This is sent by the WAL server in response to a CloseBackupRequest diff --git a/documentation/web/docs/developer/_protocol.md b/documentation/web/docs/developer/_protocol.md index 45aeaabe..806e075d 100644 --- a/documentation/web/docs/developer/_protocol.md +++ b/documentation/web/docs/developer/_protocol.md @@ -352,6 +352,7 @@ been completed. | send_to_tier2 | [bool](#bool) | | Require this backup to be sent to tier2. | | tier2_retention_policy | [string](#string) | | When present, set the tier2 retention policy to the specified JSON-serialized policy. | | tier2_compression_policy | [string](#string) | | When present, set the tier2 compression policy to the specified JSON-serialized policy. | +| wait_for_wals | [bool](#bool) | | True when the client will call CloseBackup again until no WAL file is missing. The server then defers the post-backup task until that point. When false the task is enqueued right away, even if WAL files are still missing: a backup taken on a standby cannot force a WAL switch on the primary, so its last segment may take a long time to arrive. | diff --git a/documentation/web/docs/developer/running-e2e-tests.md b/documentation/web/docs/developer/running-e2e-tests.md index 9c7edc40..8ee02c12 100644 --- a/documentation/web/docs/developer/running-e2e-tests.md +++ b/documentation/web/docs/developer/running-e2e-tests.md @@ -121,8 +121,9 @@ The E2E tests are located in `operator/test/e2e/` and include: - `RecoverReplicaCluster`: replica cluster creation from backup - **`tablespace_recovery_test.go`** - Recovery preserving PostgreSQL tablespaces (`RecoverClusterWithTablespaces`) -- **`tier2_recovery_test.go`** - Recovery from tier2 S3 storage - (`RecoverClusterFromTier2`) +- **`tier2_recovery_test.go`** - Recovery from tier2 S3 storage of a + backup taken on a standby of a two-instance cluster, which must reach + tier2 without waiting for its last WAL (`RecoverClusterFromTier2`) - **`tier2_pitr_test.go`** - Point-in-time recovery from tier2 storage (`RecoverClusterFromTier2Pitr`) - **`tier2_recovery_common_test.go`** - Shared tier2 recovery helpers used diff --git a/operator/test/e2e/tier2_recovery_test.go b/operator/test/e2e/tier2_recovery_test.go index 46b72c77..ffdbff09 100644 --- a/operator/test/e2e/tier2_recovery_test.go +++ b/operator/test/e2e/tier2_recovery_test.go @@ -40,6 +40,7 @@ import ( machineryConditions "github.com/cloudnative-pg/klio/operator/test/machinery/pkg/conditions" machineryFeatures "github.com/cloudnative-pg/klio/operator/test/machinery/pkg/features" "github.com/cloudnative-pg/klio/operator/test/machinery/pkg/namespaces" + "github.com/cloudnative-pg/klio/operator/test/machinery/pkg/postgres" ) // tier2RecoveryScenario contains all resources needed for tier2 recovery testing. @@ -168,6 +169,18 @@ func (s *tier2RecoveryScenario) Teardown( return ctx } +// switchWALOnSource forces a WAL switch on the source primary after the +// backup. A backup taken on a standby cannot switch the WAL itself, so +// without this the segment holding its end LSN would never be completed on +// an idle cluster, and the recovery would wait for it forever. +func (s *tier2RecoveryScenario) switchWALOnSource( + ctx context.Context, + _ *cnpgv1.Cluster, + r *resources.Resources, +) error { + return postgres.CheckpointAndSwitchWal(ctx, r, &s.sourcePrimaryPod) +} + // deployRecoveryServer creates the second Klio Server after tier2 replication. func (s *tier2RecoveryScenario) deployRecoveryServer( ctx context.Context, @@ -220,21 +233,27 @@ func NewTier2RecoveryFeatureConfig( } recoveryConfig := machineryFeatures.RecoveryFeatureConfig{ - Name: name, - Setup: scenario.Setup, - Teardown: scenario.Teardown, - SourcePrimaryPod: &scenario.sourcePrimaryPod, - Backup: res.Backup, - RecoveryCluster: res.RecoveryCluster, - MutateRecoveryCluster: []machineryFeatures.RecoveryClusterMutateFunc{scenario.deployRecoveryServer}, - BackupTimeout: 5 * time.Minute, + Name: name, + Setup: scenario.Setup, + Teardown: scenario.Teardown, + SourcePrimaryPod: &scenario.sourcePrimaryPod, + Backup: res.Backup, + RecoveryCluster: res.RecoveryCluster, + MutateRecoveryCluster: []machineryFeatures.RecoveryClusterMutateFunc{ + scenario.switchWALOnSource, + scenario.deployRecoveryServer, + }, + BackupTimeout: 5 * time.Minute, } return recoveryConfig } // RecoverClusterFromTier2 returns a RecoveryFeature for tier2 recovery testing. +// The source cluster has two instances so that the backup, with the default +// prefer-standby target, is taken on the standby: such a backup does not wait +// for its last WAL segment and must still be relayed to tier2. func RecoverClusterFromTier2(namespace string) *machineryFeatures.RecoveryFeature { return machineryFeatures.NewRecoveryFeature( - NewTier2RecoveryFeatureConfig("RecoverClusterFromTier2", 1, namespace)) + NewTier2RecoveryFeatureConfig("RecoverClusterFromTier2", 2, namespace)) } From 85a814e269784b5f5852a4977489a755b64c98bb Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Tue, 15 Sep 2026 16:14:09 +0200 Subject: [PATCH 2/3] fix: make the CloseBackup wire default keep the WAL wait 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 --- core/cmd/backup/run.go | 2 +- core/internal/grpc/klio_wal.pb.go | 24 +++++++++---------- core/internal/server/walserver/backup.go | 12 +++++----- core/internal/server/walserver/backup_test.go | 22 ++++++++--------- core/proto/klio_wal.proto | 10 ++++---- documentation/web/docs/developer/_protocol.md | 2 +- 6 files changed, 36 insertions(+), 36 deletions(-) diff --git a/core/cmd/backup/run.go b/core/cmd/backup/run.go index 17919c8b..6e4bd13e 100644 --- a/core/cmd/backup/run.go +++ b/core/cmd/backup/run.go @@ -158,7 +158,7 @@ func runBackup(cmd *cobra.Command, _ []string) error { EndWal: metadata.EndWAL, SegmentSize: metadata.SegmentSize, SendToTier2: tier2, - WaitForWals: waitWALs, + EnqueueWithoutWals: !waitWALs, Tier2RetentionPolicy: marshalTier2RetentionPolicy(cmd.Context(), &configuration), Tier2CompressionPolicy: marshalTier2CompressionPolicy(cmd.Context(), &configuration), }) diff --git a/core/internal/grpc/klio_wal.pb.go b/core/internal/grpc/klio_wal.pb.go index 158c03b6..6e24e8a4 100644 --- a/core/internal/grpc/klio_wal.pb.go +++ b/core/internal/grpc/klio_wal.pb.go @@ -730,14 +730,14 @@ type CloseBackupRequest struct { Tier2RetentionPolicy string `protobuf:"bytes,9,opt,name=tier2_retention_policy,json=tier2RetentionPolicy,proto3" json:"tier2_retention_policy,omitempty"` // When present, set the tier2 compression policy to the specified JSON-serialized policy. Tier2CompressionPolicy string `protobuf:"bytes,10,opt,name=tier2_compression_policy,json=tier2CompressionPolicy,proto3" json:"tier2_compression_policy,omitempty"` - // True when the client will call CloseBackup again until no WAL file is - // missing. The server then defers the post-backup task until that point. - // When false the task is enqueued right away, even if WAL files are still - // missing: a backup taken on a standby cannot force a WAL switch on the + // Enqueue the post-backup task even if WAL files are still missing. A + // client that leaves this unset calls CloseBackup again until no WAL file + // is missing, and the server defers the task until that call. A backup + // taken on a standby sets it: the standby cannot force a WAL switch on the // primary, so its last segment may take a long time to arrive. - WaitForWals bool `protobuf:"varint,11,opt,name=wait_for_wals,json=waitForWals,proto3" json:"wait_for_wals,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + EnqueueWithoutWals bool `protobuf:"varint,11,opt,name=enqueue_without_wals,json=enqueueWithoutWals,proto3" json:"enqueue_without_wals,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *CloseBackupRequest) Reset() { @@ -833,9 +833,9 @@ func (x *CloseBackupRequest) GetTier2CompressionPolicy() string { return "" } -func (x *CloseBackupRequest) GetWaitForWals() bool { +func (x *CloseBackupRequest) GetEnqueueWithoutWals() bool { if x != nil { - return x.WaitForWals + return x.EnqueueWithoutWals } return false } @@ -943,7 +943,7 @@ const file_proto_klio_wal_proto_rawDesc = "" + "\fStartWALFile\x12!\n" + "\fklio_version\x18\x01 \x01(\x04R\vklioVersion\x12\x1f\n" + "\vfile_length\x18\x02 \x01(\x04R\n" + - "fileLength\"\x85\x03\n" + + "fileLength\"\x93\x03\n" + "\x12CloseBackupRequest\x12!\n" + "\fcluster_name\x18\x01 \x01(\tR\vclusterName\x12\x1f\n" + "\vbackup_name\x18\x03 \x01(\tR\n" + @@ -955,8 +955,8 @@ const file_proto_klio_wal_proto_rawDesc = "" + "\rsend_to_tier2\x18\b \x01(\bR\vsendToTier2\x124\n" + "\x16tier2_retention_policy\x18\t \x01(\tR\x14tier2RetentionPolicy\x128\n" + "\x18tier2_compression_policy\x18\n" + - " \x01(\tR\x16tier2CompressionPolicy\x12\"\n" + - "\rwait_for_wals\x18\v \x01(\bR\vwaitForWals\"f\n" + + " \x01(\tR\x16tier2CompressionPolicy\x120\n" + + "\x14enqueue_without_wals\x18\v \x01(\bR\x12enqueueWithoutWals\"f\n" + "\x11CloseBackupResult\x12%\n" + "\x0etier2_schedule\x18\x01 \x01(\bR\rtier2Schedule\x12*\n" + "\x11missing_wal_files\x18\x02 \x03(\tR\x0fmissingWalFiles2\xd8\x03\n" + diff --git a/core/internal/server/walserver/backup.go b/core/internal/server/walserver/backup.go index 00913920..b42b3979 100644 --- a/core/internal/server/walserver/backup.go +++ b/core/internal/server/walserver/backup.go @@ -45,12 +45,12 @@ func (w *Implementation) CloseBackup( return nil, err } - // A client that waits for the WALs calls CloseBackup again until none is - // missing, so the task is deferred to that call. A client that does not - // wait (a backup taken on a standby, which cannot force a WAL switch on - // the primary) calls only once: the task must be enqueued now or the - // backup is never processed. - if len(missingWALFiles) > 0 && request.GetWaitForWals() { + // By default a client calls CloseBackup again until no WAL is missing, so + // the task is deferred to that call. A client that does not wait (a + // backup taken on a standby, which cannot force a WAL switch on the + // primary) calls only once and asks to enqueue the task right away, or + // the backup would never be processed. + if len(missingWALFiles) > 0 && !request.GetEnqueueWithoutWals() { return &grpc.CloseBackupResult{ Tier2Schedule: false, MissingWalFiles: missingWALFiles, diff --git a/core/internal/server/walserver/backup_test.go b/core/internal/server/walserver/backup_test.go index cb1b27f9..8efbb154 100644 --- a/core/internal/server/walserver/backup_test.go +++ b/core/internal/server/walserver/backup_test.go @@ -107,16 +107,16 @@ func receiveBackupTask(t *testing.T, q *queue.Conn, timeout time.Duration) *queu } } -func newCloseBackupRequest(waitForWALs bool) *grpc.CloseBackupRequest { +func newCloseBackupRequest(enqueueWithoutWALs bool) *grpc.CloseBackupRequest { return &grpc.CloseBackupRequest{ - ClusterName: testClusterName, - BackupName: "backup-1", - Timeline: 1, - StartWal: testStartWAL, - EndWal: testEndWAL, - SegmentSize: testSegmentSize, - SendToTier2: true, - WaitForWals: waitForWALs, + ClusterName: testClusterName, + BackupName: "backup-1", + Timeline: 1, + StartWal: testStartWAL, + EndWal: testEndWAL, + SegmentSize: testSegmentSize, + SendToTier2: true, + EnqueueWithoutWals: enqueueWithoutWALs, } } @@ -126,7 +126,7 @@ func newCloseBackupRequest(waitForWALs bool) *grpc.CloseBackupRequest { func TestCloseBackupMissingWALsWithoutWaitEnqueuesTask(t *testing.T) { impl, q := newCloseBackupServer(t) - result, err := impl.CloseBackup(context.Background(), newCloseBackupRequest(false)) + result, err := impl.CloseBackup(context.Background(), newCloseBackupRequest(true)) require.NoError(t, err) require.Equal(t, []string{testEndWAL}, result.GetMissingWalFiles()) require.True(t, result.GetTier2Schedule()) @@ -143,7 +143,7 @@ func TestCloseBackupMissingWALsWithoutWaitEnqueuesTask(t *testing.T) { func TestCloseBackupMissingWALsWithWaitDefersTask(t *testing.T) { impl, q := newCloseBackupServer(t) - result, err := impl.CloseBackup(context.Background(), newCloseBackupRequest(true)) + result, err := impl.CloseBackup(context.Background(), newCloseBackupRequest(false)) require.NoError(t, err) require.Equal(t, []string{testEndWAL}, result.GetMissingWalFiles()) require.False(t, result.GetTier2Schedule()) diff --git a/core/proto/klio_wal.proto b/core/proto/klio_wal.proto index 718bd5fa..81b9b2fe 100644 --- a/core/proto/klio_wal.proto +++ b/core/proto/klio_wal.proto @@ -165,12 +165,12 @@ message CloseBackupRequest { // When present, set the tier2 compression policy to the specified JSON-serialized policy. string tier2_compression_policy = 10; - // True when the client will call CloseBackup again until no WAL file is - // missing. The server then defers the post-backup task until that point. - // When false the task is enqueued right away, even if WAL files are still - // missing: a backup taken on a standby cannot force a WAL switch on the + // Enqueue the post-backup task even if WAL files are still missing. A + // client that leaves this unset calls CloseBackup again until no WAL file + // is missing, and the server defers the task until that call. A backup + // taken on a standby sets it: the standby cannot force a WAL switch on the // primary, so its last segment may take a long time to arrive. - bool wait_for_wals = 11; + bool enqueue_without_wals = 11; } // This is sent by the WAL server in response to a CloseBackupRequest diff --git a/documentation/web/docs/developer/_protocol.md b/documentation/web/docs/developer/_protocol.md index 806e075d..cb5a574c 100644 --- a/documentation/web/docs/developer/_protocol.md +++ b/documentation/web/docs/developer/_protocol.md @@ -352,7 +352,7 @@ been completed. | send_to_tier2 | [bool](#bool) | | Require this backup to be sent to tier2. | | tier2_retention_policy | [string](#string) | | When present, set the tier2 retention policy to the specified JSON-serialized policy. | | tier2_compression_policy | [string](#string) | | When present, set the tier2 compression policy to the specified JSON-serialized policy. | -| wait_for_wals | [bool](#bool) | | True when the client will call CloseBackup again until no WAL file is missing. The server then defers the post-backup task until that point. When false the task is enqueued right away, even if WAL files are still missing: a backup taken on a standby cannot force a WAL switch on the primary, so its last segment may take a long time to arrive. | +| enqueue_without_wals | [bool](#bool) | | Enqueue the post-backup task even if WAL files are still missing. A client that leaves this unset calls CloseBackup again until no WAL file is missing, and the server defers the task until that call. A backup taken on a standby sets it: the standby cannot force a WAL switch on the primary, so its last segment may take a long time to arrive. | From 0844b69c1a654a44312ca3f73c9298191e981b31 Mon Sep 17 00:00:00 2001 From: Gabriele Fedi Date: Wed, 16 Sep 2026 17:38:17 +0200 Subject: [PATCH 3/3] docs: describe enqueue_without_wals without tying it to standbys 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 --- core/internal/grpc/klio_wal.pb.go | 5 ++--- core/internal/server/walserver/backup.go | 7 +++---- core/internal/server/walserver/backup_test.go | 8 ++++---- core/proto/klio_wal.proto | 5 ++--- documentation/web/docs/developer/_protocol.md | 2 +- 5 files changed, 12 insertions(+), 15 deletions(-) diff --git a/core/internal/grpc/klio_wal.pb.go b/core/internal/grpc/klio_wal.pb.go index 6e24e8a4..cb429dbe 100644 --- a/core/internal/grpc/klio_wal.pb.go +++ b/core/internal/grpc/klio_wal.pb.go @@ -732,9 +732,8 @@ type CloseBackupRequest struct { Tier2CompressionPolicy string `protobuf:"bytes,10,opt,name=tier2_compression_policy,json=tier2CompressionPolicy,proto3" json:"tier2_compression_policy,omitempty"` // Enqueue the post-backup task even if WAL files are still missing. A // client that leaves this unset calls CloseBackup again until no WAL file - // is missing, and the server defers the task until that call. A backup - // taken on a standby sets it: the standby cannot force a WAL switch on the - // primary, so its last segment may take a long time to arrive. + // is missing, and the server defers the task until that call. A client can + // request an immediate enqueue of the post-backup task by setting this to true. EnqueueWithoutWals bool `protobuf:"varint,11,opt,name=enqueue_without_wals,json=enqueueWithoutWals,proto3" json:"enqueue_without_wals,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache diff --git a/core/internal/server/walserver/backup.go b/core/internal/server/walserver/backup.go index b42b3979..8cbbca86 100644 --- a/core/internal/server/walserver/backup.go +++ b/core/internal/server/walserver/backup.go @@ -46,10 +46,9 @@ func (w *Implementation) CloseBackup( } // By default a client calls CloseBackup again until no WAL is missing, so - // the task is deferred to that call. A client that does not wait (a - // backup taken on a standby, which cannot force a WAL switch on the - // primary) calls only once and asks to enqueue the task right away, or - // the backup would never be processed. + // the task is deferred to that call. A client may not wait for the WALs to be archived, + // and can set EnqueueWithoutWals to true in the request + // to force the backup to be enqueued even if some WALs are missing. if len(missingWALFiles) > 0 && !request.GetEnqueueWithoutWals() { return &grpc.CloseBackupResult{ Tier2Schedule: false, diff --git a/core/internal/server/walserver/backup_test.go b/core/internal/server/walserver/backup_test.go index 8efbb154..e1845437 100644 --- a/core/internal/server/walserver/backup_test.go +++ b/core/internal/server/walserver/backup_test.go @@ -120,8 +120,8 @@ func newCloseBackupRequest(enqueueWithoutWALs bool) *grpc.CloseBackupRequest { } } -// TestCloseBackupMissingWALsWithoutWaitEnqueuesTask covers a backup taken on -// a standby: the client does not wait for the last WAL, so the post-backup +// TestCloseBackupMissingWALsWithoutWaitEnqueuesTask covers a backup taken +// with a client that does not wait for the last WAL to be archived, so the post-backup // task must be enqueued on this single call or the backup is never relayed. func TestCloseBackupMissingWALsWithoutWaitEnqueuesTask(t *testing.T) { impl, q := newCloseBackupServer(t) @@ -137,8 +137,8 @@ func TestCloseBackupMissingWALsWithoutWaitEnqueuesTask(t *testing.T) { require.True(t, task.SendToTier2) } -// TestCloseBackupMissingWALsWithWaitDefersTask covers a backup taken on the -// primary: the client retries CloseBackup until no WAL is missing, so the +// TestCloseBackupMissingWALsWithWaitDefersTask covers a backup taken with a client +// that waits for the last WAL to be archived: the client retries CloseBackup until no WAL is missing, so the // task must not be enqueued before then. func TestCloseBackupMissingWALsWithWaitDefersTask(t *testing.T) { impl, q := newCloseBackupServer(t) diff --git a/core/proto/klio_wal.proto b/core/proto/klio_wal.proto index 81b9b2fe..e2f013e0 100644 --- a/core/proto/klio_wal.proto +++ b/core/proto/klio_wal.proto @@ -167,9 +167,8 @@ message CloseBackupRequest { // Enqueue the post-backup task even if WAL files are still missing. A // client that leaves this unset calls CloseBackup again until no WAL file - // is missing, and the server defers the task until that call. A backup - // taken on a standby sets it: the standby cannot force a WAL switch on the - // primary, so its last segment may take a long time to arrive. + // is missing, and the server defers the task until that call. A client can + // request an immediate enqueue of the post-backup task by setting this to true. bool enqueue_without_wals = 11; } diff --git a/documentation/web/docs/developer/_protocol.md b/documentation/web/docs/developer/_protocol.md index cb5a574c..dc375548 100644 --- a/documentation/web/docs/developer/_protocol.md +++ b/documentation/web/docs/developer/_protocol.md @@ -352,7 +352,7 @@ been completed. | send_to_tier2 | [bool](#bool) | | Require this backup to be sent to tier2. | | tier2_retention_policy | [string](#string) | | When present, set the tier2 retention policy to the specified JSON-serialized policy. | | tier2_compression_policy | [string](#string) | | When present, set the tier2 compression policy to the specified JSON-serialized policy. | -| enqueue_without_wals | [bool](#bool) | | Enqueue the post-backup task even if WAL files are still missing. A client that leaves this unset calls CloseBackup again until no WAL file is missing, and the server defers the task until that call. A backup taken on a standby sets it: the standby cannot force a WAL switch on the primary, so its last segment may take a long time to arrive. | +| enqueue_without_wals | [bool](#bool) | | Enqueue the post-backup task even if WAL files are still missing. A client that leaves this unset calls CloseBackup again until no WAL file is missing, and the server defers the task until that call. A client can request an immediate enqueue of the post-backup task by setting this to true. |