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