pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed

From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Backup command and functions can cause assertion failure and segmentation fault
Date: Fri, 1 Jul 2022 15:32:50 +0900
Message-ID: <6a759108-a02c-34fd-856c-d7c09d29d2ba@oss.nttdata.com> (raw)
In-Reply-To: <CAD21AoBkR611M6K+6i-xNDjmCd-=hFN_svNgRtvuezyjviw1sA@mail.gmail.com>
References: <3374718f-9fbf-a950-6d66-d973e027f44c@oss.nttdata.com>
	<CAD21AoBkR611M6K+6i-xNDjmCd-=hFN_svNgRtvuezyjviw1sA@mail.gmail.com>



On 2022/07/01 15:09, Masahiko Sawada wrote:
> The change looks good to me. I've also confirmed the change fixed the issues.

Thanks for the review and test!

> @@ -233,6 +233,12 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
>      StringInfo  labelfile;
>      StringInfo  tblspc_map_file;
>      backup_manifest_info manifest;
> +   SessionBackupState status = get_backup_status();
> +
> +   if (status == SESSION_BACKUP_RUNNING)
> +       ereport(ERROR,
> +               (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> +                errmsg("a backup is already in progress in this session")));
> 
> I think we can move it to the beginning of SendBaseBackup() so we can
> avoid bbsink initialization and cleanup in the error case.

Sounds good idea to me. I updated the patch in that way. Attached.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 8764084e21..c4d956b9c1 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8783,6 +8783,8 @@ do_pg_abort_backup(int code, Datum arg)
 	{
 		XLogCtl->Insert.forcePageWrites = false;
 	}
+
+	sessionBackupState = SESSION_BACKUP_NONE;
 	WALInsertLockRelease();
 
 	if (emit_warning)
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 95440013c0..637c0ce459 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -949,6 +949,12 @@ SendBaseBackup(BaseBackupCmd *cmd)
 {
 	basebackup_options opt;
 	bbsink	   *sink;
+	SessionBackupState status = get_backup_status();
+
+	if (status == SESSION_BACKUP_RUNNING)
+		ereport(ERROR,
+				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				 errmsg("a backup is already in progress in this session")));
 
 	parse_basebackup_options(cmd->options, &opt);
 


Attachments:

  [text/plain] fix_assertion_failure_and_segv_when_backup_v2.patch (1009B, ../6a759108-a02c-34fd-856c-d7c09d29d2ba@oss.nttdata.com/2-fix_assertion_failure_and_segv_when_backup_v2.patch)
  download | inline diff:
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 8764084e21..c4d956b9c1 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8783,6 +8783,8 @@ do_pg_abort_backup(int code, Datum arg)
 	{
 		XLogCtl->Insert.forcePageWrites = false;
 	}
+
+	sessionBackupState = SESSION_BACKUP_NONE;
 	WALInsertLockRelease();
 
 	if (emit_warning)
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 95440013c0..637c0ce459 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -949,6 +949,12 @@ SendBaseBackup(BaseBackupCmd *cmd)
 {
 	basebackup_options opt;
 	bbsink	   *sink;
+	SessionBackupState status = get_backup_status();
+
+	if (status == SESSION_BACKUP_RUNNING)
+		ereport(ERROR,
+				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				 errmsg("a backup is already in progress in this session")));
 
 	parse_basebackup_options(cmd->options, &opt);
 

view thread (18+ messages)  latest in thread

Message-ID: <6a759108-a02c-34fd-856c-d7c09d29d2ba@oss.nttdata.com>
Permalink:  ../6a759108-a02c-34fd-856c-d7c09d29d2ba@oss.nttdata.com/
Also on:    postgresql.org/message-id/6a759108-a02c-34fd-856c-d7c09d29d2ba@oss.nttdata.com

copy link · copy postgr.es · copy git am

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: masao.fujii@oss.nttdata.com, sawada.mshk@gmail.com, pgsql-hackers@lists.postgresql.org
  Subject: Re: Backup command and functions can cause assertion failure and segmentation fault
  In-Reply-To: <6a759108-a02c-34fd-856c-d7c09d29d2ba@oss.nttdata.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox