Hi Hou zj,
Thanks for the patch. The fix looks correct and the approach mirrors the one taken in commit 2a5225b for the xmin race.
I have a question about copy_replication_slot() in slotfuncs.c, the restart_lsn is written under only the spinlock, followed by ReplicationSlotsComputeRequiredLSN() the same pattern that this patch fixes. Could this path be affected by the same race?
Looking at the code, I think it is safe because create_logical_replication_slot() is called with src_restart_lsn which is always a valid LSN (if not valid, an error is thrown). Inside CreateInitDecodingContext(), since restart_lsn is valid, ReplicationSlotReserveWal() is skipped and the slot's restart_lsn is set directly to src_restart_lsn. So by the time the write of the slot happens, the destination slot already has a valid non-zero restart_lsn, the InvalidXLogRecPtr window never exists. Additionally, line the code errors out if copy_restart_lsn < src_restart_lsn, so the write never moves restart_lsn backward, meaning a concurrent scanner will always see a valid LSN and never skip this slot.
If that reasoning is correct, a comment near the ReplicationSlotsComputeRequiredLSN() in copy_replication_slot() explaining why it does not need the same protection would help future readers.
Regards,
Surya Poondla
