| 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
| 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] |