On Thu, Oct 14, 2021 at 02:58:55PM +0530, Dilip Kumar wrote: > On Thu, Oct 14, 2021 at 12:24 PM Michael Paquier <mich...@paquier.xyz> wrote: >> Yes, you are right here. I did not remember the semantics this relies >> on. I have played more with the patch, reviewed the whole, and the >> fields you are resetting as part of the snapshot builds seem correct >> to me. So let's fix this. > > Great, thanks!
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 :) 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. -- Michael
signature.asc
Description: PGP signature
