Re: logical decoding and replication of sequences, take 2

On Mon, Nov 27, 2023 at 11:34 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Mon, Nov 27, 2023 at 6:41 AM Tomas Vondra
> <tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
> >
> > While going over 0001, I realized there might be an optimization for
> > ReorderBufferSequenceIsTransactional. As coded in 0001, it always
> > searches through all top-level transactions, and if there's many of them
> > that might be expensive, even if very few of them have any relfilenodes
> > in the hash table. It's still linear search, and it needs to happen for
> > each sequence change.
> >
> > But can the relfilenode even be in some other top-level transaction? How
> > could it be - our transaction would not see it, and wouldn't be able to
> > generate the sequence change. So we should be able to simply check *our*
> > transaction (or if it's a subxact, the top-level transaction). Either
> > it's there (and it's transactional change), or not (and then it's
> > non-transactional change).
> >
>
> I also think the relfilenode should be part of either the current
> top-level xact or one of its subxact, so looking at all the top-level
> transactions for each change doesn't seem advisable.
>
> > The 0004 does this.
> >
> > This of course hinges on when exactly the transactions get created, and
> > assignments processed. For example if this would fire before the txn
> > gets assigned to the top-level one, this would break. I don't think this
> > can happen thanks to the immediate logging of assignments, but I'm too
> > tired to think about it now.
> >
>
> This needs some thought because I think we can't guarantee the
> association till we reach the point where we can actually decode the
> xact. See comments in AssertTXNLsnOrder() [1].
>

I am wondering that instead of building the infrastructure to know
whether a particular change is transactional on the decoding side,
can't we have some flag in the WAL record to note whether the change
is transactional or not? I have discussed this point with my colleague
Kuroda-San and we thought that it may be worth exploring whether we
can use rd_createSubid/rd_newRelfilelocatorSubid in RelationData to
determine if the sequence is created/changed in the current
subtransaction and then record that in WAL record. By this, we need to
have additional information in the WAL record like XLOG_SEQ_LOG but we
can probably do it only with wal_level as logical.

One minor point:
It'd also
+ * trigger assert in DecodeSequence.

I don't see DecodeSequence() in the patch. Which exact assert/function
are you referring to here?

--
With Regards,
Amit Kapila.