Re: logical decoding and replication of sequences, take 2

From: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, 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-04-05 21:26:33
Message-ID: ae53cd57-fcfe-7b49-6133-641f10f76103@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 4/5/23 12:39, Alvaro Herrera wrote:
> Patch 0002 is very annoying to scroll, and I realized that it's because
> psql is writing 200kB of dashes in one of the test_decoding test cases.
> I propose to set psql's printing format to 'unaligned' to avoid that,
> which should cut the size of that patch to a tenth.
>

Yeah, that's a good idea, I think. It shrunk the diff to ~90kB, which is
much better.

> I wonder if there's a similar issue in 0003, but I didn't check.
>

I don't think so, there just seems to be enough code changes to generate
~260kB diff with all the context.

As for the cfbot failures reported by Greg, that turned out to be a
minor thinko in the protocol version negotiation, introduced by part
0008 (current part, after adding Alvaro's patch tweaking test output).
The subscriber failed to send 'sequences on' when starting the stream.
It also forgot to refresh the subscription after a sequence was added.

The attached patch version fixes all of this, but I think at this point
it's better to just postpone this for PG17 - if it was something we
could fix within a single release, maybe. But the replication protocol
is something we can't easily change after release, so if we find out the
versioning (and sequence negotiation) should work differently, we can't
change it. In fact, we'd be probably stuck with it until PG16 gets out
of support, not just until PG17 ...

I've thought about pushing at least the first two parts (adding the
sequence decoding infrastructure and test_decoding support), but I'm not
sure that's quite worth it without the built-in replication stuff.

Or we could push it and then tweak it after feature freeze, if we
conclude the protocol versioning should work differently. I recall we
did changes in the column and row filtering in PG15. But that seems
quite wrong, obviously.

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment Content-Type Size
0001-Logical-decoding-of-sequences-20230405.patch text/x-patch 49.1 KB
0002-make-test_decoding-ddl.out-shorter-20230405.patch text/x-patch 473.1 KB
0003-Add-decoding-of-sequences-to-test_decoding-20230405.patch text/x-patch 87.7 KB
0004-Add-decoding-of-sequences-to-built-in-repli-20230405.patch text/x-patch 258.3 KB
0005-add-interlock-with-ALTER-SEQUENCE-20230405.patch text/x-patch 3.6 KB
0006-Support-LOCK-for-sequences-instead-of-funct-20230405.patch text/x-patch 3.9 KB
0007-Reconstruct-the-right-state-from-the-on-dis-20230405.patch text/x-patch 1.4 KB
0008-protocol-changes-20230405.patch text/x-patch 15.1 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2023-04-05 21:27:14 Re: [PATCH] Add `verify-system` sslmode to use system CA pool for server cert
Previous Message David Rowley 2023-04-05 21:23:31 Re: Negative cache entries for memoize