Re: logical decoding and replication of sequences, take 2

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
Cc: John Naylor <john(dot)naylor(at)enterprisedb(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Heikki Linnakangas <heikki(dot)linnakangas(at)iki(dot)fi>
Subject: Re: logical decoding and replication of sequences, take 2
Date: 2023-03-16 07:38:07
Message-ID: CAD21AoBzuKt6ogcD+7YAjL_ep8mSxifHDf+ZVGhLP-Pkuxej0A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Wed, Mar 15, 2023 at 9:52 PM Tomas Vondra
<tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
>
>
>
> On 3/14/23 08:30, John Naylor wrote:
> > I tried a couple toy examples with various combinations of use styles.
> >
> > Three with "automatic" reading from sequences:
> >
> > create table test(i serial);
> > create table test(i int GENERATED BY DEFAULT AS IDENTITY);
> > create table test(i int default nextval('s1'));
> >
> > ...where s1 has some non-default parameters:
> >
> > CREATE SEQUENCE s1 START 100 MAXVALUE 100 INCREMENT BY -1;
> >
> > ...and then two with explicit use of s1, one inserting the 'nextval'
> > into a table with no default, and one with no table at all, just
> > selecting from the sequence.
> >
> > The last two seem to work similarly to the first three, so it seems like
> > FOR ALL TABLES adds all sequences as well. Is that expected?
>
> Yeah, that's a bug - we shouldn't replicate the sequence changes, unless
> the sequence is actually added to the publication. I tracked this down
> to a thinko in get_rel_sync_entry() which failed to check the object
> type when puballtables or puballsequences was set.
>
> Attached is a patch fixing this.
>
> > The documentation for CREATE PUBLICATION mentions sequence options,
> > but doesn't really say how these options should be used.
> Good point. The idea is that we handle tables and sequences the same
> way, i.e. if you specify 'sequence' then we'll replicate increments for
> sequences explicitly added to the publication.
>
> If this is not clear, the docs may need some improvements.
>

I'm late to this thread, but I have some questions and review comments.

Regarding sequence logical replication, it seems that changes of
sequence created after CREATE SUBSCRIPTION are applied on the
subscriber even without REFRESH PUBLICATION command on the subscriber.
Which is a different behavior than tables. For example, I set both
publisher and subscriber as follows:

1. On publisher
create publication test_pub for all sequences;

2. On subscriber
create subscription test_sub connection 'dbname=postgres port=5551'
publication test_pub; -- port=5551 is the publisher

3. On publisher
create sequence s1;
select nextval('s1');

I got the error "ERROR: relation "public.s1" does not exist on the
subscriber". Probably we need to do should_apply_changes_for_rel()
check in apply_handle_sequence().

If my understanding is correct, is there any case where the subscriber
needs to apply transactional sequence changes? The commit message of
0001 patch says:

* Changes for sequences created in the same top-level transaction are
treated as transactional, i.e. just like any other change from that
transaction, and discarded in case of a rollback.

IIUC such sequences are not visible to the subscriber, so it cannot
subscribe to them until the commit.

---
I got an assertion failure. The reproducible steps are:

1. On publisher
alter system set logical_replication_mode = 'immediate';
select pg_reload_conf();
create publication test_pub for all sequences;

2. On subscriber
create subscription test_sub connection 'dbname=postgres port=5551'
publication test_pub with (streaming='parall\el')

3. On publisher
begin;
create table bar (c int, d serial);
insert into bar(c) values (100);
commit;

I got the following assertion failure:

TRAP: failed Assert("(!seq.transactional) || in_remote_transaction"),
File: "worker.c", Line: 1458, PID: 508056
postgres: logical replication parallel apply worker for subscription
16388 (ExceptionalCondition+0x9e)[0xb6c0af]
postgres: logical replication parallel apply worker for subscription
16388 [0x92f7fe]
postgres: logical replication parallel apply worker for subscription
16388 (apply_dispatch+0xed)[0x932925]
postgres: logical replication parallel apply worker for subscription
16388 [0x90d927]
postgres: logical replication parallel apply worker for subscription
16388 (ParallelApplyWorkerMain+0x34f)[0x90dd8d]
postgres: logical replication parallel apply worker for subscription
16388 (StartBackgroundWorker+0x1f3)[0x8e7b19]
postgres: logical replication parallel apply worker for subscription
16388 [0x8f1798]
postgres: logical replication parallel apply worker for subscription
16388 [0x8f1b53]
postgres: logical replication parallel apply worker for subscription
16388 [0x8f0bed]
postgres: logical replication parallel apply worker for subscription
16388 [0x8ecca4]
postgres: logical replication parallel apply worker for subscription
16388 (PostmasterMain+0x1246)[0x8ec6d7]
postgres: logical replication parallel apply worker for subscription
16388 [0x7bbe5c]
/lib64/libc.so.6(__libc_start_main+0xf3)[0x7f69094cbcf3]
postgres: logical replication parallel apply worker for subscription
16388 (_start+0x2e)[0x49d15e]
2023-03-16 12:33:19.471 JST [507974] LOG: background worker "logical
replication parallel worker" (PID 508056) was terminated by signal 6:
Aborted

seq.transactional is true and in_remote_transaction is false. It might
be an issue of the parallel apply feature rather than this patch.

---
There is no documentation about the new 'sequence' value of the
publish option in CREATE/ALTER PUBLICATION. It seems to be possible to
specify something like "CREATE PUBLICATION ... FOR ALL SEQUENCES WITH
(publish = 'truncate')" (i.e., not specifying 'sequence' value in the
publish option). How does logical replication work with this setting?
Nothing is replicated?

---
It seems that sequence replication does't work well together with
ALTER SUBSCRIPTION ... SKIP command. IIUC these changes are not
skipped even if these are transactional changes. The reproducible
steps are:

1. On both nodes
create table a (c int primary key);

2. On publisher
create publication hoge_pub for all sequences, tables

3. On subscriber
create subscription hoge_sub connection 'dbname=postgres port=5551'
publication hoge_pub;
insert into a values (1);

4. On publisher
begin;
create sequence s2;
insert into a values (nextval('s2'));
commit;

At step 4, applying INSERT conflicts with the existing row on the
subscriber. If I skip this transaction using ALTER SUBSCRIPTION ...
SKIP command, I got:

ERROR: relation "public.s2" does not exist
CONTEXT: processing remote data for replication origin "pg_16390"
during message type "BEGIN" in transaction 734, finished at 0/1751698

If I create the sequence s2 in advance on the subscriber, the sequence
change is applied on the subscriber.

If the subscriber doesn't need to apply transactional sequence changes
in the first place, this problem will disappear.

---
There are two typos in 0001 patch:

In the commit message:

ensure the sequence record has a valid XID - until now the the increment

s/the the/ the/

And,

+ /* Only ever called from ReorderBufferApplySequence, so transational. */

s/transational/transactional/

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2023-03-16 07:40:33 Remove last traces of SCM credential auth from libpq?
Previous Message Michael Paquier 2023-03-16 07:18:12 Re: Combine pg_walinspect till_end_of_wal functions with others