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