Re: logical decoding: skip unnecessary snapshot distribution. - Mailing list pgsql-hackers

From Shlok Kyal
Subject Re: logical decoding: skip unnecessary snapshot distribution.
Date
Msg-id CANhcyEVDozpvCfQv0_E1F1z44W5JkFub6G=tU1FJ+wA1X+srRA@mail.gmail.com
Whole thread
In response to Re: logical decoding: skip unnecessary snapshot distribution.  (Shlok Kyal <shlok.kyal.oss@gmail.com>)
List pgsql-hackers
On Wed, 9 Sept 2026 at 13:22, 杨伯宇(长堂) <yangboyu.yby@alibaba-inc.com> wrote:
>
> Hi Shlok,
> Thanks for your review.
> > S1: BEGIN;
> > S1: INSERT INTO t1 VALUES(11);
> > S2: BEGIN;
> > S2: ALTER TABLE t2 RENAME TO t2_new;
> > S2: SAVEPOINT s1;
> > S2: ALTER ROLE role1 RENAME TO role1_new;
> > S2 ROLLBACK TO SAVEPOINT s1;
> > S2: COMMIT;
> >
> > Now I debugged DecodeCommit for transaction in S2,
> > It displayed parsed->nmsg = 10.
> > Now I continued debugging and in 'SnapBuildDistributeSnapshotAndInval', we have:
> > ninvalidations = ReorderBufferGetInvalidations(builder->reorder, xid, &msgs);
> > Here, invalidations = 13.
> > The first 10 invalidation messages in msgs were the same as
> > parsed->msgs, but there were 3 additional messages. These additional
> > messages had dbId = 0, indicating shared-catalog invalidations (I
> > assume it is due to ALTER ROLE command).
> >
> > With the patch, InvalidationsTouchSharedCatalog(parsed->nmsgs,
> > parsed->msgs) would therefore see only the first 10 messages, return
> > false, and distribute would be set to false. The 3 shared-catalog
> > invalidations returned by ReorderBufferGetInvalidations() would
> > consequently not be distributed.
> > Is this expected behavior? Thoughts?
> Yes, it's expected.
> parsed->msgs comes from the COMMIT WAL record, which carries only the
> invalidation messages that survive to commit. When a subtransaction
> aborts, AtEOSubXact_Inval(false) drops its messages instead of
> propagating them to the parent. That is why parsed->nmsgs is 10.
> ReorderBufferGetInvalidations(), on the other hand, returns the
> accumulated set of the top-level transaction. While decoding an
> XLOG_XACT_INVALIDATIONS record, its messages are merged into the
> top-level transaction's invalidation array (rbtxn_get_toptxn() in
> ReorderBufferAddInvalidations()), and that merge is irreversible: when
> the subtransaction's abort record is later decoded,
> ReorderBufferAbort() only cleans up the subtransaction's own reorder
> buffer entry and cannot retract the messages already merged into the
> top-level one. That is why it returns 13.
> So, not distributing the invalidations of an aborted subxact
> causes no correctness issue in this patch, because those catalog
> changes were never committed.

Hi Boyu,
Thanks for the detailed explanation. The explanation makes sense to me.

I have one more doubt:
For a SHAREDINVALRELCACHE_ID message, dbId == InvalidOid and
relId == InvalidOid mean "invalidate the whole relcache".  They do not mean
that a shared relation changed.  CacheInvalidateRelcacheAll() creates this
message for publication DDL, such as ALTER PUBLICATION ... SET ALL TABLES.

InvalidationsTouchSharedCatalog() returns true for this message.  Therefore,
for a commit from another database, 'distribute' stays true and the snapshot
and invalidations are still distributed.

Since this message alone does not mean that a shared catalog changed, should
we ignore it when deciding whether to distribute changes from another
database?

Thanks,
Shlok Kyal



pgsql-hackers by date:

Previous
From: Peter Eisentraut
Date:
Subject: Declare variable-length catalog columns as [] rather than [1]
Next
From: Alexandre Felipe
Date:
Subject: Re: aio: worker: Free SMGR objects when idle