Re: Reset snapshot export state on the transaction abort

On Sat, Oct 16, 2021 at 08:31:36AM -0700, Zhihong Yu wrote:
> On Sat, Oct 16, 2021 at 3:10 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>> On Sat, Oct 16, 2021 at 9:13 AM Michael Paquier <michael@paquier.xyz>
>> wrote:
>>> 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.

That seems logically fine.  I'll check that tomorrow.

> +extern void ResetSnapBuildExportSnapshotState(void);
> 
> ResetSnapBuildExportSnapshotState() is only called inside snapbuild.c
> I wonder if the addition to snapbuild.h is needed.

As of xact.c in v2 of the patch, we have that:
@@ -2698,6 +2699,9 @@ AbortTransaction(void)
        /* Reset logical streaming state. */
	        ResetLogicalStreamingState();

+       /* Reset snapshot export state. */
+       ResetSnapBuildExportSnapshotState();
--
Michael