On Tue, Mar 30, 2021 at 12:00 PM Markus Wanner
<markus.wanner@enterprisedb.com> wrote:
>
> Hello Ajin,
>
> On 30.03.21 06:48, Ajin Cherian wrote:
> > For now, I've created a patch that addresses the problem reported using
> > the existing callbacks.
>
> Thanks.
>
> > Do have a look if this fixes the problem reported.
>
> Yes, this replaces the PREPARE I would do from the concurrent_abort
> callback in a direct call to rb->prepare.
>
That sounds clearly a better choice. Because concurrent_abort()
internally trying to prepare transaction seems a bit ugly and not only
that if we want to go via that route, it needs to distinguish between
rollback to savepoint and rollback cases as well.
Now, we can try to find a way where for such cases we don't send
prepare/rollback prepare, but somehow detect it and send rollback
instead. And also some more checks will be required so that if we have
streamed the transaction then send stream_abort. I am not telling that
all this is not possible but I don't find worth making all such
checks.
> However, it misses the most
> important part: documentation. Because this clearly is a surprising
> behavior for a transaction that's not fully decoded and guaranteed to
> get aborted.
>
Yeah, I guess that makes sense to me. I think we can document it in
the docs [1] where we explained two-phase decoding. I think we can add
a point about concurrent aborts at the end of points mentioned in the
following paragraph: "The users that want to decode prepared
transactions need to be careful ....."
[1] - https://www.postgresql.org/docs/devel/logicaldecoding-two-phase-commits.html
--
With Regards,
Amit Kapila.
