On Wed, 9 Sept 2026 at 13:22, 杨伯宇(长堂) <[email protected]> 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
