Content-Type: multipart/mixed; boundary="------------avCMetnnSAkqo3zkCP7pEppT"
Message-ID: <6a759108-a02c-34fd-856c-d7c09d29d2ba@oss.nttdata.com>
Date: Fri, 1 Jul 2022 15:32:50 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.11.0
Subject: Re: Backup command and functions can cause assertion failure and
 segmentation fault
Content-Language: en-US
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
References: <3374718f-9fbf-a950-6d66-d973e027f44c@oss.nttdata.com>
 <CAD21AoBkR611M6K+6i-xNDjmCd-=hFN_svNgRtvuezyjviw1sA@mail.gmail.com>
From: Fujii Masao <masao.fujii@oss.nttdata.com>
In-Reply-To: 
 <CAD21AoBkR611M6K+6i-xNDjmCd-=hFN_svNgRtvuezyjviw1sA@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/6a759108-a02c-34fd-856c-d7c09d29d2ba%40oss.nttdata.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------avCMetnnSAkqo3zkCP7pEppT
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


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
--------------avCMetnnSAkqo3zkCP7pEppT
Content-Type: text/plain; charset=UTF-8;
 name="fix_assertion_failure_and_segv_when_backup_v2.patch"
Content-Disposition: attachment;
 filename="fix_assertion_failure_and_segv_when_backup_v2.patch"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL2FjY2Vzcy90cmFuc2FtL3hsb2cuYyBiL3NyYy9i
YWNrZW5kL2FjY2Vzcy90cmFuc2FtL3hsb2cuYwppbmRleCA4NzY0MDg0ZTIxLi5jNGQ5NTZi
OWMxIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9hY2Nlc3MvdHJhbnNhbS94bG9nLmMKKysr
IGIvc3JjL2JhY2tlbmQvYWNjZXNzL3RyYW5zYW0veGxvZy5jCkBAIC04NzgzLDYgKzg3ODMs
OCBAQCBkb19wZ19hYm9ydF9iYWNrdXAoaW50IGNvZGUsIERhdHVtIGFyZykKIAl7CiAJCVhM
b2dDdGwtPkluc2VydC5mb3JjZVBhZ2VXcml0ZXMgPSBmYWxzZTsKIAl9CisKKwlzZXNzaW9u
QmFja3VwU3RhdGUgPSBTRVNTSU9OX0JBQ0tVUF9OT05FOwogCVdBTEluc2VydExvY2tSZWxl
YXNlKCk7CiAKIAlpZiAoZW1pdF93YXJuaW5nKQpkaWZmIC0tZ2l0IGEvc3JjL2JhY2tlbmQv
cmVwbGljYXRpb24vYmFzZWJhY2t1cC5jIGIvc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vYmFz
ZWJhY2t1cC5jCmluZGV4IDk1NDQwMDEzYzAuLjYzN2MwY2U0NTkgMTAwNjQ0Ci0tLSBhL3Ny
Yy9iYWNrZW5kL3JlcGxpY2F0aW9uL2Jhc2ViYWNrdXAuYworKysgYi9zcmMvYmFja2VuZC9y
ZXBsaWNhdGlvbi9iYXNlYmFja3VwLmMKQEAgLTk0OSw2ICs5NDksMTIgQEAgU2VuZEJhc2VC
YWNrdXAoQmFzZUJhY2t1cENtZCAqY21kKQogewogCWJhc2ViYWNrdXBfb3B0aW9ucyBvcHQ7
CiAJYmJzaW5rCSAgICpzaW5rOworCVNlc3Npb25CYWNrdXBTdGF0ZSBzdGF0dXMgPSBnZXRf
YmFja3VwX3N0YXR1cygpOworCisJaWYgKHN0YXR1cyA9PSBTRVNTSU9OX0JBQ0tVUF9SVU5O
SU5HKQorCQllcmVwb3J0KEVSUk9SLAorCQkJCShlcnJjb2RlKEVSUkNPREVfT0JKRUNUX05P
VF9JTl9QUkVSRVFVSVNJVEVfU1RBVEUpLAorCQkJCSBlcnJtc2coImEgYmFja3VwIGlzIGFs
cmVhZHkgaW4gcHJvZ3Jlc3MgaW4gdGhpcyBzZXNzaW9uIikpKTsKIAogCXBhcnNlX2Jhc2Vi
YWNrdXBfb3B0aW9ucyhjbWQtPm9wdGlvbnMsICZvcHQpOwogCg==

--------------avCMetnnSAkqo3zkCP7pEppT--