Horton

Re: logical decoding and replication of sequences

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>, Petr Jelinek <petr.jelinek@enterprisedb.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-04-01T05:41:26Z
Lists: pgsql-hackers 
On Fri, Apr 1, 2022 at 10:22 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Sat, Mar 26, 2022 at 6:56 PM Tomas Vondra
> <tomas.vondra@enterprisedb.com> wrote:
> > >>
> > >> But there's a more serious issue, I think. So far, we allowed this:
> > >>
> > >>   BEGIN;
> > >>   CREATE SEQUENCE s2;
> > >>   ALTER PUBLICATION p ADD SEQUENCE s2;
> > >>   INSERT INTO seq_test SELECT nextval('s2') FROM generate_series(1,100);
> > >>   COMMIT;
> > >>
> > >> and the behavior was that we replicated the changes. But with the patch
> > >> applied, that no longer happens, because should_apply_changes_for_rel
> > >> says the change should not be applied.
> > >>
> > >> And after thinking about this, I think that's correct - we can't apply
> > >> changes until ALTER SUBSCRIPTION ... REFRESH PUBLICATION gets executed,
> > >> and we can't do that until the transaction commits.
> > >>
> > >> So I guess that's correct, and the current behavior is a bug.
> > >>
> > >
> > > Yes, I also think that is a bug.
> > >
> >
> > OK
>
> I also think that this is a bug. Given this behavior is a bug and
> newly-added sequence data should be replicated only after ALTER
> SUBSCRIPTION ... REFRESH PUBLICATION, is there any case where the
> sequence message applied on the subscriber is transactional?
>

It could be required for Alter Sequence as that can also rewrite the
relfilenode. However, IIUC, I think there is a bigger problem with
non-transactional sequence implementation as that can cause
inconsistent replica. See the problem description and test case in my
previous email [1] (While thinking about this, I think I see a problem
with the non-transactional handling of sequences....). Can you please
once check that and let me know if I am missing something there? If
not, then I think we may need to first think of a solution for
non-transactional sequence handling.

[1] - https://www.postgresql.org/message-id/CAA4eK1KAFdQEULk%2B4C%3DieWA5UPSUtf_gtqKsFj9J9f2c%3D8hm4g%40mail.gmail.com

-- 
With Regards,
Amit Kapila.

Commits

  1. Fix cache invalidation bug in recovery_prefetch.

  2. Revert "Logical decoding of sequences"

  3. Minor improvements in sequence decoding code and docs

  4. Handle sequences in preprocess_pubobj_list

  5. Update tab-completion for CREATE PUBLICATION with sequences

  6. Add decoding of sequences to built-in replication

  7. Stabilize test_decoding touching with sequences

  8. Call ReorderBufferProcessXid from sequence_decode

  9. Add decoding of sequences to test_decoding

  10. Replace Test::More plans with done_testing

  11. Logical decoding of sequences

  12. Respect permissions within logical replication.