Re: Reset snapshot export state on the transaction abort

On Wed, Oct 13, 2021 at 10:17 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Mon, Oct 11, 2021 at 08:46:32PM +0530, Dilip Kumar wrote:
> > As reported at [1], if the transaction is aborted during export
> > snapshot then ExportInProgress and SavedResourceOwnerDuringExport are
> > not getting reset and that is throwing an error
> > "clearing exported snapshot in wrong transaction state" while
> > executing the next command. The attached patch clears this state if
> > the transaction is aborted.

Correct.

> Injecting an error is enough to reproduce the failure in a second
> command after the first one failed. This could happen on OOM for the
> palloc() done at the beginning of SnapBuildInitialSnapshot().
>
> @@ -2698,6 +2698,9 @@ AbortTransaction(void)
> /* Reset logical streaming state. */
> ResetLogicalStreamingState();
>
> + /* Reset snapshot export state. */
> + ResetSnapBuildExportSnapshotState();
> Shouldn't we care about the case of a sub-transaction abort as well?
> See AbortSubTransaction().

Actually, it is not required because 1) Snapshot export can not be
allowed within a transaction block, basically, it starts its own
transaction block and aborts that while executing any next replication
command see SnapBuildClearExportedSnapshot(). So our problem is only
if the transaction block internally started for exporting, gets
aborted before any next command arrives. So there is no possibility
of starting any sub transaction.

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