Re: logical decoding: skip unnecessary snapshot distribution.

From: Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>
To: 杨伯宇(长堂) <yangboyu(dot)yby(at)alibaba-inc(dot)com>
Cc: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, rhaas <rhaas(at)postgresql(dot)org>, shihao zhong <zhong950419(at)gmail(dot)com>
Subject: Re: logical decoding: skip unnecessary snapshot distribution.
Date: 2026-09-22 07:51:51
Message-ID: CANhcyEVDozpvCfQv0_E1F1z44W5JkFub6G=tU1FJ+wA1X+srRA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, 9 Sept 2026 at 13:22, 杨伯宇(长堂) <yangboyu(dot)yby(at)alibaba-inc(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexandre Felipe 2026-09-22 07:54:22 Re: aio: worker: Free SMGR objects when idle
Previous Message Peter Eisentraut 2026-09-22 07:47:35 Declare variable-length catalog columns as [] rather than [1]