Re: repeated decoding of prepared transactions

On Mon, Feb 8, 2021 at 2:01 PM Markus Wanner
<markus.wanner@enterprisedb.com> wrote:
>
> Amit, Ajin, hackers,
>
> testing logical decoding for two-phase transactions, I stumbled over
> what I first thought is a bug.  But comments seems to indicate this is
> intended behavior.  Could you please clarify or elaborate on the design
> decision?  Or indicate this indeed is a bug?
>
> What puzzled me is that if a decoder is restarted in between the PREPARE
> and the COMMIT PREPARED, it repeats the entire transaction, despite it
> being already sent and potentially prepared on the receiving side.
>
> In terms of `pg_logical_slot_get_changes` (and roughly from the
> prepare.sql test), this looks as follows:
>
>                          data
> ----------------------------------------------------
>   BEGIN
>   table public.test_prepared1: INSERT: id[integer]:1
>   PREPARE TRANSACTION 'test_prepared#1'
> (3 rows)
>
>
> This is the first delivery of the transaction.  After a restart, it will
> get all of the changes again, though:
>
>
>                          data
> ----------------------------------------------------
>   BEGIN
>   table public.test_prepared1: INSERT: id[integer]:1
>   PREPARE TRANSACTION 'test_prepared#1'
>   COMMIT PREPARED 'test_prepared#1'
> (4 rows)
>
>
> I did not expect this, as any receiver that wants to have decoded 2PC is
> likely supporting some kind of two-phase commits itself.  And would
> therefore prepare the transaction upon its first reception.  Potentially
> receiving it a second time would require complicated filtering on every
> prepared transaction.
>

The reason was mentioned in ReorderBufferFinishPrepared(). See below
comments in code:
/*
 * It is possible that this transaction is not decoded at prepare time
 * either because by that time we didn't have a consistent snapshot or it
 * was decoded earlier but we have restarted. We can't distinguish between
 * those two cases so we send the prepare in both the cases and let
 * downstream decide whether to process or skip it. We don't need to
 * decode the xact for aborts if it is not done already.
 */
This won't happen when we replicate via pgoutput  (the patch for which
is still not committed) because it won't restart from a previous point
(unless the server needs to be restarted due to some reason) as you
are doing via logical decoding APIs. Now, we don't send again the
prepared xacts on repeated calls of pg_logical_slot_get_changes()
unless we encounter commit. This behavior is already explained in docs
[1] (See, Transaction Begin Prepare Callback in docs) and a way to
skip the prepare.

> Furthermore, this clearly and unnecessarily holds back the restart LSN.
> Meaning even just a single prepared transaction can block advancing the
> restart LSN.  In most cases, these are short lived.  But on the other
> hand, there may be an arbitrary amount of other transactions in between
> a PREPARE and the corresponding COMMIT PREPARED in the WAL.  Not being
> able to advance over a prepared transaction seems like a bad thing in
> such a case.
>

That anyway is true without this work as well where restart_lsn can be
advanced on commits. We haven't changed anything in that regard.

[1] - https://www.postgresql.org/docs/devel/logicaldecoding-output-plugin.html


-- 
With Regards,
Amit Kapila.