Re: Logical Replication WIP

From: Stas Kelvich <s(dot)kelvich(at)postgrespro(dot)ru>
To: Petr Jelinek <petr(at)2ndquadrant(dot)com>
Cc: Craig Ringer <craig(at)2ndquadrant(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Logical Replication WIP
Date: 2016-08-11 11:34:30
Message-ID: BD75F8C5-E3D5-4B52-9D30-BBCCE0150687@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 05 Aug 2016, at 18:00, Petr Jelinek <petr(at)2ndquadrant(dot)com> wrote:
>
> Hi,
>
> as promised here is WIP version of logical replication patch.

Great!

Proposed DDL about publication/subscriptions looks very nice to me.

Some notes and thoughts about patch:

* Clang grumbles at following pieces of code:

apply.c:1316:6: warning: variable 'origin_startpos' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]

tablesync.c:436:45: warning: if statement has empty body [-Wempty-body]
if (wait_for_sync_status_change(tstate));

* max_logical_replication_workers mentioned everywhere in docs, but guc.c defines
variable called max_logical_replication_processes for postgresql.conf

* Since pg_subscription already shared across the cluster, it can be also handy to
share pg_publications too and allow publication of tables from different databases. That
is rare scenarios but quite important for virtual hosting use case — tons of small databases
in a single postgres cluster.

* There is no way to see attachecd tables/schemas to publication through \drp

* As far as I understand there is no way to add table/tablespace right in CREATE
PUBLICATION and one need explicitly do ALTER PUBLICATION right after creation.
May be add something like WITH TABLE/TABLESPACE to CREATE?

* So binary protocol goes into core. Is it still possible to use it as decoding plugin for
manually created walsender? May be also include json as it was in pglogical? While
i’m not arguing that it should be done, i’m interested about your opinion on that.

* Also I’ve noted that you got rid of reserved byte (flags) in protocol comparing to
pglogical_native. It was very handy to use it for two phase tx decoding (0 — usual
commit, 1 — prepare, 2 — commit prepared), because both prepare and commit
prepared generates commit record in xlog.

> On 05 Aug 2016, at 18:00, Petr Jelinek <petr(at)2ndquadrant(dot)com> wrote:
>
> - DDL, I see several approaches we could do here for 10.0. a) don't
> deal with DDL at all yet, b) provide function which pushes the DDL
> into replication queue and then executes on downstream (like
> londiste, slony, pglogical do), c) capture the DDL query as text
> and allow user defined function to be called with that DDL text on
> the subscriber

* Since here DDL is mostly ALTER / CREATE / DROP TABLE (or am I wrong?) may be
we can add something like WITH SUBSCRIBERS to statements?

* Talking about exact mechanism of DDL replication I like you variant b), but since we
have transactional DDL, we can do two phase commit here. That will require two phase
decoding and some logic about catching prepare responses through logical messages. If that
approach sounds interesting i can describe proposal in more details and create a patch.

* Also I wasn’t able actually to run replication itself =) While regression tests passes, TAP
tests and manual run stuck. pg_subscription_rel.substate never becomes ‘r’. I’ll investigate
that more and write again.

* As far as I understand sync starts automatically on enabling publication. May be split that
logic into a different command with some options? Like don’t sync at all for example.

* When I’m trying to create subscription to non-existent publication, CREATE SUBSRITION
creates replication slot and do not destroys it:

# create subscription sub connection 'host=127.0.0.1 dbname=postgres' publication mypub;
NOTICE: created replication slot "sub" on provider
ERROR: could not receive list of replicated tables from the provider: ERROR: cache lookup failed for publication 0
CONTEXT: slot "sub", output plugin "pgoutput", in the list_tables callback

after that:

postgres=# drop subscription sub;
ERROR: subscription "sub" does not exist
postgres=# create subscription sub connection 'host=127.0.0.1 dbname=postgres' publication pub;
ERROR: could not crate replication slot "sub": ERROR: replication slot "sub" already exists

* Also can’t drop subscription:

postgres=# \drs
List of subscriptions
Name | Database | Enabled | Publication | Conninfo
------+----------+---------+-------------+--------------------------------
sub | postgres | t | {mypub} | host=127.0.0.1 dbname=postgres
(1 row)

postgres=# drop subscription sub;
ERROR: unrecognized object class: 6102

* Several time i’ve run in a situation where provider's postmaster ignores Ctrl-C until subscribed
node is switched off.

* Patch with small typos fixed attached.

I’ll do more testing, just want to share what i have so far.

Attachment Content-Type Size
typos.diff application/octet-stream 2.0 KB
unknown_filename text/plain 95 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David G. Johnston 2016-08-11 12:34:44 Re: Surprising behaviour of \set AUTOCOMMIT ON
Previous Message Palle Girgensohn 2016-08-11 11:22:09 Re: Improved ICU patch - WAS: Implementing full UTF-8 support (aka supporting 0x00)