Re: logical decoding and replication of sequences

On Fri, Mar 25, 2022 at 3:29 AM Tomas Vondra
<tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
>
> Pushed.
>

Some of the comments given by me [1] don't seem to be addressed or
responded to. Let me try to say again for the ease of discussion:

* Don't we need some syncing mechanism between apply worker and
sequence sync worker so that apply worker skips the sequence changes
till the sync worker is finished, otherwise, there is a risk of one
overriding the values of the other? See how we take care of this for a
table in should_apply_changes_for_rel() and its callers. If we don't
do this for sequences for some reason then probably a comment
somewhere is required.

* Don't we need explicit privilege checking before applying sequence
data as we do in commit a2ab9c06ea15fbcb2bfde570986a06b37f52bcca for
tables?

Few new comments:
=================
1. A simple test like the below crashes for me:
postgres=# create sequence s1;
CREATE SEQUENCE
postgres=# create sequence s2;
CREATE SEQUENCE
postgres=# create publication pub1 for sequence s1, s2;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.

2. In apply_handle_sequence() do we need AccessExclusiveLock for
non-transactional case?

3. In apply_handle_sequence(), I think for transactional case, we need
to skip the operation, if the skip lsn is set. See how we skip in
apply_handle_insert() and similar functions.

[1] - https://www.postgresql.org/message-id/CAA4eK1Jn-DttQ%3D4Pdh9YCe1w%2BzGbgC%2B0uR0sfg2RtkjiAPmB9g%40mail.gmail.com

--
With Regards,
Amit Kapila.