On Sat, Oct 16, 2021 at 3:10 AM Dilip Kumar <dilipbal...@gmail.com> wrote:
> On Sat, Oct 16, 2021 at 9:13 AM Michael Paquier <mich...@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. > > > > -- > Regards, > Dilip Kumar > EnterpriseDB: http://www.enterprisedb.com Hi, bq. While exporting a snapshot we set a temporary states which get a temporary states -> temporary states +extern void ResetSnapBuildExportSnapshotState(void); ResetSnapBuildExportSnapshotState() is only called inside snapbuild.c I wonder if the addition to snapbuild.h is needed. Cheers
