Re: logical decoding and replication of sequences

On Sat, Apr 2, 2022 at 8:52 PM Tomas Vondra
<tomas.vondra@enterprisedb.com> wrote:
>
>
>
> On 4/2/22 12:35, Amit Kapila wrote:
> > On Fri, Apr 1, 2022 at 8:32 PM Tomas Vondra
> > <tomas.vondra@enterprisedb.com> wrote:
> >>
> >> On 3/28/22 07:29, Amit Kapila wrote:
> >>> I thought about changing snapshot dealing of
> >>> non-transactional sequence changes similar to transactional ones but
> >>> that also won't work because it is only at commit we decide whether we
> >>> can send the changes.
> >>>
> >> I wonder if there's some earlier LSN (similar to the consistent point)
> >> which might be useful for this.
> >>
> >> Or maybe we should queue even the non-transactional changes, not
> >> per-transaction but in a global list, and then at each commit either
> >> discard inspect them (at that point we know the lowest LSN for all
> >> transactions and the consistent point). Seems complex, though.
> >>
> >
> > I couldn't follow '..discard inspect them ..'. Do you mean we inspect
> > them and discard whichever are not required? It seems here we are
> > talking about a new global ReorderBufferGlobal instead of
> > ReorderBufferTXN to collect these changes but we don't need only
> > consistent point LSN because we do send if the commit of containing
> > transaction is after consistent point LSN, so we need some transaction
> > information as well. I think it could bring new challenges.
> >
>
> Sorry for the gibberish. Yes, I meant to discard sequence changes that
> are no longer needed, due to being "obsoleted" by the applied change. We
> must not apply "older" changes (using LSN) because that would make the
> sequence go backwards.

It's not related to this issue but I think that non-transactional
sequence changes could be resent in case of the subscriber crashes
because it doesn’t update replication origin LSN, is that right? If
so, while resending the sequence changes, the sequence value on the
subscriber can temporarily go backward.

>
> I'm not entirely sure whether the list of changes should be kept in TXN
> or in the global reorderbuffer object - we need to track which TXN the
> change belongs to (because of transactional changes) but we also need to
> discard the unnecessary changes efficiently (and walking TXN might be
> expensive).
>
> But yes, I'm sure there will be challenges. One being that tracking just
> the decoded WAL stuff is not enough, because nextval() may not generate
> WAL. But we still need to make sure the increment is replicated.
>
> What I think we might do is this:
>
> - add a global list of decoded sequence increments to ReorderBuffer
>
> - at each commit/abort walk the list, walk the list and consider all
> increments up to the commit LSN that "match" (non-transactional match
> all TXNs, transactional match only the current TXN)
>
> - replicate the last "matching" status for each sequence, discard the
> processed ones
>
> We could probably optimize this by not tracking every single increment,
> but merge them "per transaction", I think.
>
> I'm sure this description is pretty rough and will need refining, handle
> various corner-cases etc.
>
> >>> For the transactional case, as we are considering the create sequence
> >>> operation as transactional, we would unnecessarily queue them even
> >>> though that is not required. Basically, they don't need to be
> >>> considered transactional and we can simply ignore such messages like
> >>> other DDLs. But for that probably we need to distinguish Alter/Create
> >>> case which may or may not be straightforward. Now, queuing them is
> >>> probably harmless unless it causes the transaction to spill/stream.
> >>>
> >>
> >> I'm not sure I follow. Why would we queue them unnecessarily?
> >>
> >> Also, there's the bug with decoding changes in transactions that create
> >> the sequence and add it to a publication. I think the agreement was that
> >> this behavior was incorrect, we should not decode changes until the
> >> subscription is refreshed. Doesn't that mean can't be any CREATE case,
> >> just ALTER?
> >>
> >
> > Yeah, but how will we distinguish them. Aren't they using the same
> > kind of WAL record?
> >
>
> Same WAL record, but the "created" flag which should distinguish these
> two cases, IIRC.

Since the "created" flag indicates that we created a new relfilenode
so it's true when both CREATE and ALTER.

Regards,

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/