Re: logical decoding and replication of sequences, take 2

From: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
Subject: Re: logical decoding and replication of sequences, take 2
Date: 2023-07-12 19:05:15
Message-ID: 09daff6f-8e62-115c-3a5c-4bae54ddda70@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

here's a rebased and significantly reworked version of this patch
series, based on the recent reviews and discussion. Let me go through
the main differences:

1) reorder the patches to have the "shortening" of test output first

2) merge the various "fix" patches in to the three main patches

0002 - introduce sequence decoding infrastructure
0003 - add sequences to test_decoding
0004 - add sequences to built-in replication

I've kept those patches separate to make the evolution easier to follow
and discuss, but it was necessary to cleanup the patch series and make
it clearer what the current state is.

3) simplify the replicated state

As suggested by Ashutosh, it may not be a good idea to replicate the
(last_value, log_cnt, is_called) tuple, as that's pretty tightly tied to
our internal implementation. Which may not be the right thing for other
plugins. So this new patch replicates just "value" which is pretty much
(last_value + log_cnt), representing the next value that should be safe
to generate on the subscriber (in case of a failover).

4) simplify test_decoding code & tests

I realized I can ditch some of the test_decoding changes, because at
some point we chose to only include sequences in test_decoding when
explicitly requested. So the tests don't need to disable that, it's the
other way - one test needs to enable it.

This now also prints the single value, instead of the three values.

5) minor tweaks in the built-in replication

This adopts the relaxed LOCK code to allow locking sequences during the
initial sync, and also adopts the replication of a single value (this
affects the "apply" side of that change too).

6) simplified protocol versioning

The main open question I had was what to do about protocol versioning
for the built-in replication - how to decide whether the subscriber can
apply sequences, and what should happen if we decode sequence but the
subscriber does not support that.

I was not entirely sure we want to handle this by a simple version
check, because that maps capabilities to a linear scale, which seems
pretty limiting. That is, each protocol version just grows, and new
version number means support of a new capability - like replication of
two-phase commits, or sequences. Which is nice, but it does not allow
supporting just the later feature, for example - you can't skip one.
Which is why 2PC decoding has both a version and a subscription flag,
which allows exactly that ...

When discussing this off-list with Peter Eisentraut, he reminded me of
his old message in the thread:

https://www.postgresql.org/message-id/8046273f-ea88-5c97-5540-0ccd5d244fd4@enterprisedb.com

where he advocates for exactly this simplified behavior. So I took a
stab at it and 0005 should be doing that. I keep it as a separate patch
for now, to make the changes clearer, but ultimately it should be merged
into 0003 and 0004 parts.

It's not particularly complex change, it mostly ditches the subscription
option (which also means columns in the pg_subscription catalog), and a
flag in the decoding context.

But the main change is in pgoutput_sequence(), where we protocol_version
and error-out if it's not the right version (instead of just ignoring
the sequence). AFAICS this behaves as expected - with PG15 subscriber, I
get an ERROR on the publisher side from the sequence callback.

But it no occurred to me we could do the same thing with the original
approach - allow the per-subscription "sequences" flag, but error out
when the subscriber did not enable that capability ...

Hopefully, I haven't forgotten to address any important point from the
reviews ...

The one thing I'm not really sure about is how it interferes with the
replication of DDL. But in principle, if it decodes DDL for ALTER
SEQUENCE, I don't see why it would be a problem that we then decode and
replicate the WAL for the sequence state. But if it is a problem, we
should be able to skip this WAL record with the initial sequence state
(which I think should be possible thanks to the "created" flag this
patch adds to the WAL record).

regards

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

Attachment Content-Type Size
0001-Make-test_decoding-ddl.out-shorter-20230712.patch text/x-patch 473.3 KB
0002-Logical-decoding-of-sequences-20230712.patch text/x-patch 51.8 KB
0003-Add-decoding-of-sequences-to-test_decoding-20230712.patch text/x-patch 20.6 KB
0004-Add-decoding-of-sequences-to-built-in-repli-20230712.patch text/x-patch 268.0 KB
0005-Simplify-protocol-versioning-20230712.patch text/x-patch 15.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joe Conway 2023-07-12 19:09:24 Re: COPY table FROM STDIN via SPI
Previous Message chap 2023-07-12 18:43:21 Re: COPY table FROM STDIN via SPI