On Sat, Oct 16, 2021 at 9:13 AM Michael Paquier <michael@paquier.xyz> wrote: > > While double-checking this stuff, I have noticed something that's > wrong in the patch when a command that follows a > CREATE_REPLICATION_SLOT query resets SnapBuildClearExportedSnapshot(). > Once the slot is created, the WAL sender is in a TRANS_INPROGRESS > state, meaning that AbortCurrentTransaction() would call > AbortTransaction(), hence calling ResetSnapBuildExportSnapshotState() > and resetting SavedResourceOwnerDuringExport to NULL before we store a > NULL into CurrentResourceOwner :)
Right, good catch!
> One solution would be as simple as saving > SavedResourceOwnerDuringExport into a temporary variable before > calling AbortCurrentTransaction(), and save it back into > CurrentResourceOwner once we are done in > SnapBuildClearExportedSnapshot() as we need to rely on > AbortTransaction() to do the static state cleanup if an error happens > until the command after the replslot creation command shows up.
Yeah, this idea looks fine to me. I have modified the patch. In addition to that I have removed calling ResetSnapBuildExportSnapshotState from the SnapBuildClearExportedSnapshot because that is anyway being called from the AbortTransaction.
