Re: Reset snapshot export state on the transaction abort

On Sat, Oct 16, 2021 at 9:13 AM Michael Paquier <michael(at)paquier(dot)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.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com