Re: Assertion failure in pg_copy_logical_replication_slot()

On 2020/06/24 9:38, Alvaro Herrera wrote:
> On 2020-Jun-23, Fujii Masao wrote:
>
>> 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)
>
> Oops.
>
>> 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")));
>
> Heh, you pasted the code after your patch rather than the original.

oh.... sorry.

> I think the errcode is a bit bogus considering the new case.
> IMO ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE is more appropriate.

Agreed. So I updated the patch so this errcode is used instead.
Patch attached.

> One could argue that the error message could also be different for the
> case of a logical slot (or even a physical slot that has the upcoming
> "invalidated_at" LSN set), maybe "cannot copy a replication slot that
> has been invalidated" but maybe that's a pointless distinction.
> I don't object to the patch as presented.

I have no strong opinion about this, but for now I kept the message as it is.

Regards,

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