Re: logical decoding and replication of sequences

On Wed, Dec 15, 2021 at 7:21 PM Tomas Vondra
<tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
>
> On 12/15/21 14:20, Amit Kapila wrote:
> > On Tue, Dec 14, 2021 at 7:02 AM Tomas Vondra
> > <tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
> >>
> >> Hi,
> >>
> >> here's an updated version of the patches, dealing with almost all of the
> >> issues (at least in the 0001 and 0002 parts). The main changes:
> >>
> >> 1) I've removed the 'created' flag from fill_seq_with_data, as
> >> discussed. I don't think it's needed by any of the parts (not even 0003,
> >> AFAICS). We still need it in xl_seq_rec, though.
> >>
> >> 2) GetCurrentTransactionId() added to sequence.c are called only with
> >> wal_level=logical, to minimize the overhead.
> >>
> >>
> >> There's still one remaining problem, that I already explained in [1].
> >> The problem is that with this:
> >>
> >> BEGIN;
> >> SELECT nextval('s') FROM generate_series(1,100);
> >> ROLLBACK;
> >>
> >>
> >> The root cause is that pg_current_wal_lsn() uses the LogwrtResult.Write,
> >> which is updated by XLogFlush() - but only in RecordTransactionCommit.
> >> Which makes sense, because only the committed stuff is "visible".
> >>
> >> But the non-transactional behavior of sequence decoding disagrees with
> >> this, because now some of the changes from aborted transactions may be
> >> replicated. Which means the wait_for_catchup() ends up not waiting for
> >> the sequence change to be replicated. This is an issue for tests in
> >> patch 0003, at least.
> >>
> >> My concern is this actually affects other places waiting for things
> >> getting replicated :-/
> >>
> >
> > By any chance, will this impact synchronous replication as well which
> > waits for commits to be replicated?
> >
>
> Physical or logical replication?
>

logical replication.

> Physical is certainly not replicated.
>
> For logical replication, it's more complicated.
>
> > How is this patch dealing with prepared transaction case where at
> > prepare we will send transactional changes and then later if rollback
> > prepared happens then the publisher will rollback changes related to
> > new relfilenode but subscriber would have already replayed the updated
> > seqval change which won't be rolled back?
> >
>
> I'm not sure what exact scenario you are describing, but in general we
> don't rollback sequence changes anyway, so this should not cause any
> divergence between the publisher and subscriber.
>
> Or are you talking about something like ALTER SEQUENCE? I think that
> should trigger the transactional behavior for the new relfilenode, so
> the subscriber won't see that changes.
>

Yeah, something like Alter Sequence and nextval combination. I see
that it will be handled because of the transactional behavior for the
new relfilenode as for applying each sequence change, a new
relfilenode is created. I think on apply side, the patch always
creates a new relfilenode irrespective of whether the sequence message
is transactional or not. Do we need to do that for the
non-transactional messages as well?

--
With Regards,
Amit Kapila.