Re: logical decoding of two-phase transactions - Mailing list pgsql-hackers

On 2017-03-28 15:55:15 +0100, Simon Riggs wrote:
> On 28 March 2017 at 15:38, Andres Freund <andres@anarazel.de> wrote:
> > On 2017-03-28 15:32:49 +0100, Simon Riggs wrote:
> >> On 28 March 2017 at 03:53, Craig Ringer <craig@2ndquadrant.com> wrote:
> >> > On 28 March 2017 at 09:25, Andres Freund <andres@anarazel.de> wrote:
> >> >
> >> >> If you actually need separate decoding of 2PC, then you want to wait for
> >> >> the PREPARE to be replicated.  If that replication has to wait for the
> >> >> to-be-replicated prepared transaction to commit prepared, and commit
> >> >> prepare will only happen once replication happened...
> >> >
> >> > In other words, the output plugin cannot decode a transaction at
> >> > PREPARE TRANSACTION time if that xact holds an AccessExclusiveLock on
> >> > a catalog relation we must be able to read in order to decode the
> >> > xact.
> >>
> >> Yes, I understand.
> >>
> >> The decoding plugin can choose to enable lock_timeout, or it can
> >> choose to wait for manual resolution, or it could automatically abort
> >> such a transaction to avoid needing to decode it.
> >
> > That doesn't solve the problem. You still left with replication that
> > can't progress.  I think that's completely unacceptable.  We need a
> > proper solution to this, not throw our hands up in the air and hope that
> > it's not going to hurt a whole lot of peopel.
>
> Nobody is throwing their hands in the air, nobody is just hoping. The
> concern raised is real and needs to be handled somewhere; the only
> point of discussion is where and how.


> >> I don't think its for us to say what the plugin is allowed to do. We
> >> decided on a plugin architecture, so we have to trust that the plugin
> >> author resolves the issues. We can document them so those choices are
> >> clear.
> >
> > I don't think this is "plugin architecture" related. The output pluging
> > can't do right here, this has to be solved at a higher level.
>
> That assertion is obviously false... the plugin can resolve this in
> various ways, if we allow it.

Handling it by breaking replication isn't handling it (e.g. timeouts in
decoding etc).  Handling it by rolling back *prepared* transactions
(which are supposed to be guaranteed to succeed!), isn't either.


> You can say that in your opinion you prefer to see this handled in
> some higher level way, though it would be good to hear why and how.

It's pretty obvious why: A bit of DDL by the user shouldn't lead to the
issues mentioned above.


> Bottom line here is we shouldn't reject this patch on this point,

I think it definitely has to be rejected because of that.  And I didn't
bring this up at the last minute, I repeatedly brought it up before.
Both to Craig and Stas.

One way to fix this would be to allow decoding to acquire such locks
(i.e. locks held by the prepared xact we're decoding) - there
unfortunately are some practical issues with that (e.g. the locking code
doesnt' necessarily expect a second non-exclusive locker, when there's
an exclusive one),  or we could add an exception to the locking code to
simply not acquire such locks.


> especially since any resource issue found during decoding could
> similarly prevent progress with decoding.

For example?

- Andres