Re: logical decoding and replication of sequences, take 2

On 3/29/23 11:51, Amit Kapila wrote:
> On Wed, Mar 29, 2023 at 12:04 AM Tomas Vondra
> <tomas.vondra@enterprisedb.com> wrote:
>>
>> On 3/28/23 18:34, Masahiko Sawada wrote:
>>> On Mon, Mar 27, 2023 at 11:46 PM Tomas Vondra
>>> <tomas.vondra@enterprisedb.com> wrote:
>>>>>
>>>>> 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?
>>
> 
> I think deciding anything at the publisher would be tricky but won't
> it be better if by default we disallow connection from subscriber to
> the publisher when the publisher's version is higher? And then allow
> it only based on some subscription option or maybe by default allow
> the connection to a higher version but based on option disallows the
> connection.
> 
>>
>> 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.
>>
> 
> Is this behavior of TRUNCATE known or discussed previously? I can't
> see any mention of this in the docs or commit message. I guess if we
> want to follow such behavior it should be well documented so that it
> won't be a surprise for users. I think we would face such cases in the
> future as well. One of the similar cases we are discussing for DDL
> replication where a higher version publisher could send some DDL
> syntax that lower version subscribers won't support and will lead to
> an error [1].
> 

I don't know where/how it's documented, TBH.

FWIW I agree the TRUNCATE-like behavior (failing on subscriber after
receiving unknown message type) is a bit annoying.

Perhaps it'd be reasonable to tie the "protocol version" to subscriber
capabilities, so that a protocol version guarantees what message types
the subscriber understands. So we could increment the protocol version,
check it in pgoutput_startup and then error-out in the sequence callback
if the subscriber version is too old.

That'd be nicer in the sense that we'd generate nicer error message on
the publisher, not an "unknown message type" on the subscriber. That's
doable, the main problem being it'd be inconsistent with the TRUNCATE
behavior. OTOH that was introduced in PG11, which is the oldest version
still under support ...


regards

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