To: pgsql-hackers@postgresql.org
From: Fujii Masao <masao.fujii@oss.nttdata.com>
Subject: Assertion failure in pg_copy_logical_replication_slot()
Message-ID: <f91de4fb-a7ab-b90e-8132-74796e049d51@oss.nttdata.com>
Date: Tue, 23 Jun 2020 00:17:47 +0900
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:68.0)
 Gecko/20100101 Thunderbird/68.9.0
MIME-Version: 1.0
Content-Type: multipart/mixed;
 boundary="------------9495F6578034EF6EA01CF0CC"
Content-Language: en-US
Precedence: bulk

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

Hi,

If restart_lsn of logical replication slot gets behind more than
max_slot_wal_keep_size from the current LSN, the logical replication
slot would be invalidated and its restart_lsn is reset to an invalid LSN.
If this logical replication slot with an invalid restart_lsn is specified
as the source slot in pg_copy_logical_replication_slot(), the function
causes the following assertion failure.

     TRAP: FailedAssertion("!logical_slot", File: "slotfuncs.c", Line: 727)

This assertion failure is caused by

	/* Copying non-reserved slot doesn't make sense */
	if (XLogRecPtrIsInvalid(src_restart_lsn))
		ereport(ERROR,
				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
				 errmsg("cannot copy a replication slot that doesn't reserve WAL")));

I *guess* this assertion check was added because restart_lsn should
not be invalid before. But in v13, it can be invalid thanks to max_slot_wal_keep_size.
I think that this assertion check seems useless and should be removed in v13.
Patch attached. Thought?

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

--------------9495F6578034EF6EA01CF0CC
Content-Type: text/plain; charset=UTF-8; x-mac-type="0"; x-mac-creator="0";
 name="fix_assertion_in_pg_copy_logical_replication_slot.patch"
Content-Transfer-Encoding: base64
Content-Disposition: attachment;
 filename="fix_assertion_in_pg_copy_logical_replication_slot.patch"

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3JlcGxpY2F0aW9uL3Nsb3RmdW5jcy5jIGIvc3Jj
L2JhY2tlbmQvcmVwbGljYXRpb24vc2xvdGZ1bmNzLmMKaW5kZXggMDZlNDk1NWRlNy4uYmMz
ZmNiZmY0ZCAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vc2xvdGZ1bmNz
LmMKKysrIGIvc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vc2xvdGZ1bmNzLmMKQEAgLTcyMywx
MiArNzIzLDkgQEAgY29weV9yZXBsaWNhdGlvbl9zbG90KEZ1bmN0aW9uQ2FsbEluZm8gZmNp
bmZvLCBib29sIGxvZ2ljYWxfc2xvdCkKIAogCS8qIENvcHlpbmcgbm9uLXJlc2VydmVkIHNs
b3QgZG9lc24ndCBtYWtlIHNlbnNlICovCiAJaWYgKFhMb2dSZWNQdHJJc0ludmFsaWQoc3Jj
X3Jlc3RhcnRfbHNuKSkKLQl7Ci0JCUFzc2VydCghbG9naWNhbF9zbG90KTsKIAkJZXJlcG9y
dChFUlJPUiwKIAkJCQkoZXJyY29kZShFUlJDT0RFX0ZFQVRVUkVfTk9UX1NVUFBPUlRFRCks
CiAJCQkJIGVycm1zZygiY2Fubm90IGNvcHkgYSByZXBsaWNhdGlvbiBzbG90IHRoYXQgZG9l
c24ndCByZXNlcnZlIFdBTCIpKSk7Ci0JfQogCiAJLyogT3ZlcndyaXRlIHBhcmFtcyBmcm9t
IG9wdGlvbmFsIGFyZ3VtZW50cyAqLwogCWlmIChQR19OQVJHUygpID49IDMpCg==
--------------9495F6578034EF6EA01CF0CC--