Re: logical decoding and replication of sequences, take 2

On 3/28/23 18:34, Masahiko Sawada wrote:
> On Mon, Mar 27, 2023 at 11:46 PM Tomas Vondra
> <tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
>>
>>
>>
>> On 3/27/23 03:32, Masahiko Sawada wrote:
>>> Hi,
>>>
>>> On Fri, Mar 24, 2023 at 7:26 AM Tomas Vondra
>>> <tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
>>>>
>>>> I merged the earlier "fixup" patches into the relevant parts, and left
>>>> two patches with new tweaks (deducing the corrent "WAL" state from the
>>>> current state read by copy_sequence), and the interlock discussed here.
>>>>
>>>
>>> Apart from that, how does the publication having sequences work with
>>> subscribers who are not able to handle sequence changes, e.g. in a
>>> case where PostgreSQL version of publication is newer than the
>>> subscriber? As far as I tested the latest patches, the subscriber
>>> (v15) errors out with the error 'invalid logical replication message
>>> type "Q"' when receiving a sequence change. I'm not sure it's sensible
>>> behavior. I think we should instead either (1) deny starting the
>>> replication if the subscriber isn't able to handle sequence changes
>>> and the publication includes that, or (2) not send sequence changes to
>>> such subscribers.
>>>
>>
>> I agree the "invalid message" error is not great, but it's not clear to
>> me how to do either (1). The trouble is we don't really know if the
>> publication contains (or will contain) sequences. I mean, what would
>> happen if the replication starts and then someone adds a sequence?
>>
>> For (2), I think that's not something we should do - silently discarding
>> some messages seems error-prone. If the publication includes sequences,
>> presumably the user wanted to replicate those. If they want to replicate
>> to an older subscriber, create a publication without sequences.
>>
>> Perhaps the right solution would be to check if the subscriber supports
>> replication of sequences in the output plugin, while attempting to write
>> the "Q" message. And error-out if the subscriber does not support it.
>
> It might be related to this topic; do we need to bump the protocol
> version? The commit 64824323e57d introduced new streaming callbacks
> and bumped the protocol version. I think the same seems to be true for
> this change as it adds sequence_cb callback.
>

It's not clear to me what should be the exact behavior?

I mean, imagine we're opening a connection for logical replication, and
the subscriber does not handle sequences. What should the publisher do?

(Note: The correct commit hash is 464824323e57d.)

I don't think the streaming is a good match for sequences, because of a
couple important differences ...

Firstly, streaming determines *how* the changes are replicated, not what
gets replicated. It doesn't (silently) filter out "bad" events that the
subscriber doesn't know how to apply. If the subscriber does not know
how to deal with streamed xacts, it'll still get the same changes
exactly per the publication definition.

Secondly, the default value is "streming=off", i.e. the subscriber has
to explicitly request streaming when opening the connection. And we
simply check it against the negotiated protocol version, i.e. the check
in pgoutput_startup() protects against subscriber requesting a protocol
v1 but also streaming=on.

I don't think we can/should do more check at this point - we don't know
what's included in the requested publications at that point, and I doubt
it's worth adding because we certainly can't predict if the publication
will be altered to include/decode sequences in the future.

Speaking of precedents, TRUNCATE is probably a better one, because it's
a new action and it determines *what* the subscriber can handle. But
that does exactly the thing we do for sequences - if you open a
connection from PG10 subscriber (truncate was added in PG11), and the
publisher decodes a truncate, subscriber will do:

2023-03-28 20:29:46.921 CEST [2357609] ERROR: invalid logical
replication message type "T"
2023-03-28 20:29:46.922 CEST [2356534] LOG: worker process: logical
replication worker for subscription 16390 (PID 2357609) exited with
exit code 1

I don't see why sequences should do anything else. If you need to
replicate to such subscriber, create a publication that does not have
'sequence' in the publish option ...

regards

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