On Thu, Oct 14, 2021 at 02:58:55PM +0530, Dilip Kumar wrote:
> On Thu, Oct 14, 2021 at 12:24 PM Michael Paquier <michael@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