From: | Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com> |
---|---|
To: | Arseny Sher <a(dot)sher(at)postgrespro(dot)ru>, Nikhil Sontakke <nikhils(at)2ndquadrant(dot)com> |
Cc: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Robert Haas <robertmhaas(at)gmail(dot)com>, David Steele <david(at)pgmasters(dot)net>, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Craig Ringer <craig(at)2ndquadrant(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Sokolov Yura <y(dot)sokolov(at)postgrespro(dot)ru>, Stas Kelvich <s(dot)kelvich(at)postgrespro(dot)ru>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] logical decoding of two-phase transactions |
Date: | 2019-01-25 14:26:55 |
Message-ID: | 2e1cdeb7-6940-8558-910a-9673b507bacb@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 14/01/2019 23:16, Arseny Sher wrote:
>
> Nikhil Sontakke <nikhils(at)2ndquadrant(dot)com> writes:
>
>> I'd like to believe that the latest patch set tries to address some
>> (if not all) of your concerns. Can you please take a look and let me
>> know?
>
> Hi, sure.
>
> General things:
>
> - Earlier I said that there is no point of sending COMMIT PREPARED if
> decoding snapshot became consistent after PREPARE, i.e. PREPARE hadn't
> been sent. I realized since then that such use cases actually exist:
> prepare might be copied to the replica by e.g. basebackup or something
> else earlier.
Basebackup does not copy slots though and slot should not reach
consistency until all prepared transactions are committed no?
>
> - BTW, ReorderBufferProcessXid at PREPARE should be always called
> anyway, because otherwise if xact is empty, we will not prepare it
> (and call cb), even if the output plugin asked us not to filter it
> out. However, we will call commit_prepared cb, which is inconsistent.
>
> - I find it weird that in DecodePrepare and in DecodeCommit you always
> ask the plugin whether to filter an xact, given that sometimes you
> know beforehand that you are not going to replay it: it might have
> already been replayed, might have wrong dbid, origin, etc. One
> consequence of this: imagine that notorious xact with PREPARE before
> point where snapshot became consistent and COMMIT PREPARED after that
> point. Even if filter_cb says 'I want 2PC on this xact', with current
> code it won't be replayed on PREPARE and rbxid will be destroyed with
> ReorderBufferForget. Now this xact is lost.
Yeah this is wrong.
>
> Second patch:
>
> + /* filter_prepare is optional, but requires two-phase decoding */
> + if ((ctx->callbacks.filter_prepare_cb != NULL) && (!opt->enable_twophase))
> + ereport(ERROR,
> + (errmsg("Output plugin does not support two-phase decoding, but "
> + "registered filter_prepared callback.")));
>
> I actually think that enable_twophase output plugin option is
> redundant. If plugin author wants 2PC, he just provides
> filter_prepare_cb callback and potentially others.
+1
> I also don't see much
> value in checking that exactly 0 or 3 callbacks were registred.
>
I think that check makes sense, if you support 2pc you need to register
all callbacks.
>
> Nitpicking:
>
> First patch: I still don't think that these flags need a bitmask.
Since we are discussing this, I personally prefer the bitmask here.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Heikki Linnakangas | 2019-01-25 14:33:54 | Re: Speeding up text_position_next with multibyte encodings |
Previous Message | Nick B | 2019-01-25 14:26:38 | pg_basebackup, walreceiver and wal_sender_timeout |