Re: logical decoding and replication of sequences, take 2

On Thu, Jul 20, 2023 at 10:19 PM Tomas Vondra
<tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
>
> FWIW there's two questions related to the switch to XLOG_SMGR_CREATE.
>
> 1) Does smgr_decode() need to do the same block as sequence_decode()?
>
> /* Skip the change if already processed (per the snapshot). */
> if (transactional &&
> !SnapBuildProcessChange(builder, xid, buf->origptr))
> return;
> else if (!transactional &&
> (SnapBuildCurrentState(builder) != SNAPBUILD_CONSISTENT ||
> SnapBuildXactNeedsSkip(builder, buf->origptr)))
> return;
>
> I don't think it does. Also, we don't have any transactional flag here.
> Or rather, everything is transactional ...

Right.

>
>
> 2) Currently, the sequences hash table is in reorderbuffer, i.e. global.
> I was thinking maybe we should have it in the transaction (because we
> need to do cleanup at the end). It seem a bit inconvenient, because then
> we'd need to either search htabs in all subxacts, or transfer the
> entries to the top-level xact (otoh, we already do that with snapshots),
> and cleanup on abort.
>
> What do you think?

Hash table per transaction seems saner design. Adding it to the top
level transaction should be fine. The entry will contain an XID
anyway. If we add it to every subtransaction we will need to search
hash table in each of the subtransactions when deciding whether a
sequence change is transactional or not. Top transaction is a
reasonable trade off.

--
Best Wishes,
Ashutosh Bapat