Re: [HACKERS] logical decoding of two-phase transactions

Hi,

On 2018-07-23 16:31:50 +0530, Nikhil Sontakke wrote:
> >> > That'd require that an index lookup can't crash if the corresponding
> >> > heap entry doesn't exist (etc), but that's something we need to handle
> >> > anyway.  The issue that multiple separate catalog lookups need to be
> >> > coherent (say Robert's pg_class exists, but pg_attribute doesn't
> >> > example) is solved by virtue of the the pg_attribute lookups failing if
> >> > the transaction aborted.
> > Not quite, no. Basically, in a simplified manner, the logical decoding
> > loop is like:
> >
> > while (true)
> >     record = readRecord()
> >     logical = decodeRecord()
> >
> >     PG_TRY():
> >         StartTransactionCommand();
> >
> >         switch (TypeOf(logical))
> >             case INSERT:
> >                 insert_callback(logical);
> >                 break;
> >             ...
> >
> >         CommitTransactionCommand();
> >
> >     PG_CATCH():
> >         AbortCurrentTransaction();
> >         PG_RE_THROW();
> >
> > what I'm proposing is that that various catalog access functions throw a
> > new class of error, something like "decoding aborted transactions".
> 
> When will this error be thrown by the catalog functions? How will it
> determine that it needs to throw this error?

The error check would have to happen at the end of most systable_*
functions. They'd simply do something like

if (decoding_in_progress_xact && TransactionIdDidAbort(xid_of_aborted))
   ereport(ERROR, (errcode(DECODING_ABORTED_XACT), errmsg("oops")));

i.e. check whether the transaction to be decoded still is in
progress. As that would happen before any potentially wrong result can
be returned (as the check happens at the tail end of systable_*),
there's no issue with wrong state in the syscache etc.


> >     PG_CATCH():
> >         AbortCurrentTransaction();
> >         if (errclass == DECODING_ABORTED_XACT)
> >            in_progress_xact_abort_pending = true;
> >            continue;
> >         else
> >            PG_RE_THROW();
> >
> > Now obviously that's just pseudo code with lotsa things missing, but I
> > think the basic idea should come through?
> >
> 
> How do we handle the cases where the catalog returns inconsistent data
> (without erroring out) which does not help with the ongoing decoding?
> Consider for example:

I don't think that situation exists, given the scheme described
above. That's just the point.


> BEGIN;
> /* CONSIDER T1 has one column C1 */
> ALTER TABLE T1 ADD COL c2;
> INSERT INTO TABLE T1(c2) VALUES;
> PREPARE TRANSACTION;
> 
> If we abort the above 2PC and the catalog row for the ALTER gets
> cleaned up by vacuum, then the catalog read will return us T1 with one
> column C1.

No, it'd throw an error due to the bew is-aborted check.


> The catalog scan will NOT error out but will return metadata which
> causes the insert-decoding change apply callback to error out.

Why would it not throw an error?

Greetings,

Andres Freund