Clear base backup progress on backup failure

Previously, if a base backup failed after it had started streaming
files, pg_stat_progress_basebackup could continue to show a stale
progress entry even though the backup was no longer running. This could
be observed when the client kept the replication connection open after
the error. It is normally not observable when using pg_basebackup,
because the client disconnects after the error.

The problem was that progress reporting was cleared only after
successful completion.

This commit moves the progress reporting cleanup into the progress
sink's cleanup callback so that it is cleared after both successful
and failed backups.

Backpatch to v15. v14 has the same issue, but the fix does not apply
cleanly because it lacks the base backup sink infrastructure. Since
the bug does not affect the backup itself and is normally not
observable when using pg_basebackup, skip the v14 backpatch.

Author: Chao Li <lic@highgo.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/EA1A6CD2-EFA6-462B-9A02-03003555AB4A@gmail.com
Backpatch-through: 15

diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index 9c79dadaacc556d5b12cb62c588b1fec8b43dd03..5214e7b99c7ebb8e183c51b8cf9fc6934da1139f 100644 (file)
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -676,8 +676,6 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
 
        /* clean up the resource owner we created */
        ReleaseAuxProcessResources(true);
-
-       basebackup_progress_done();
 }
 
 /*

diff --git a/src/backend/backup/basebackup_progress.c b/src/backend/backup/basebackup_progress.c
index fb9e57f04dfed655bdd9d5640a0774854f97a39b..f74459181d02b7e7d75e9255ae4457bf330377bf 100644 (file)
--- a/src/backend/backup/basebackup_progress.c
+++ b/src/backend/backup/basebackup_progress.c
@@ -38,6 +38,7 @@
 static void bbsink_progress_begin_backup(bbsink *sink);
 static void bbsink_progress_archive_contents(bbsink *sink, size_t len);
 static void bbsink_progress_end_archive(bbsink *sink);
+static void bbsink_progress_cleanup(bbsink *sink);
 
 static const bbsink_ops bbsink_progress_ops = {
        .begin_backup = bbsink_progress_begin_backup,
@@ -48,7 +49,7 @@ static const bbsink_ops bbsink_progress_ops = {
        .manifest_contents = bbsink_forward_manifest_contents,
        .end_manifest = bbsink_forward_end_manifest,
        .end_backup = bbsink_forward_end_backup,
-       .cleanup = bbsink_forward_cleanup
+       .cleanup = bbsink_progress_cleanup
 };
 
 /*
@@ -184,6 +185,16 @@ bbsink_progress_archive_contents(bbsink *sink, size_t len)
        pgstat_progress_update_multi_param(nparam, index, val);
 }
 
+/*
+ * Clean up progress reporting.
+ */
+static void
+bbsink_progress_cleanup(bbsink *sink)
+{
+       pgstat_progress_end_command();
+       bbsink_forward_cleanup(sink);
+}
+
 /*
  * Advertise that we are waiting for the start-of-backup checkpoint.
  */
@@ -236,12 +247,3 @@ basebackup_progress_transfer_wal(void)
        pgstat_progress_update_param(PROGRESS_BASEBACKUP_PHASE,
                                                                 PROGRESS_BASEBACKUP_PHASE_TRANSFER_WAL);
 }
-
-/*
- * Advertise that we are no longer performing a backup.
- */
-void
-basebackup_progress_done(void)
-{
-       pgstat_progress_end_command();
-}

diff --git a/src/include/backup/basebackup_sink.h b/src/include/backup/basebackup_sink.h
index bbf4de0cbdcb379b1c56ddc79680e8e60ce766e9..96fa2c4eaba7fbb9f3bce0d16ae42a529b4af87d 100644 (file)
--- a/src/include/backup/basebackup_sink.h
+++ b/src/include/backup/basebackup_sink.h
@@ -297,6 +297,5 @@ extern void basebackup_progress_wait_checkpoint(void);
 extern void basebackup_progress_estimate_backup_size(void);
 extern void basebackup_progress_wait_wal_archive(bbsink_state *);
 extern void basebackup_progress_transfer_wal(void);
-extern void basebackup_progress_done(void);
 
 #endif