Re: logical decoding: skip unnecessary snapshot distribution.

From: Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>
To: yangboyu <yangboyu(dot)yby(at)alibaba-inc(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, rhaas <rhaas(at)postgresql(dot)org>
Subject: Re: logical decoding: skip unnecessary snapshot distribution.
Date: 2026-09-03 09:09:07
Message-ID: CANhcyEV=jQCqGJHV6QMM3JP+FdSvHoU9SJxgmohe9=MSUd9uZg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, 31 Aug 2026 at 12:33, yangboyu <yangboyu(dot)yby(at)alibaba-inc(dot)com> wrote:
>
> Hi hackers,
>
> I recently hit a disk bloat issue caused by logical decoding:
> (1) A cluster has two databases, db1 and db2. There is a single
> logical replication slot on db1 while db2 holds plenty of tables.
> (2) A long-running transaction sits open in db2, pinning the xmin of
> the historic snapshots during decoding.
> (3) db2's tables get ANALYZEd -- manual run over all tables.
> Each analyzed table commits separately and writes pg_statistic,
> pg_statistic_ext and pg_class row, so every such commit is a
> catalog-changing commit carrying invalidation messages.
> (4) Every one of these commits makes the decoding session of db1's
> slot rebuild its historic snapshot and distribute it to every
> in-progress transaction in its reorder buffer -- including db2's
> long transaction, whose changes the slot will never decode however.
> Since xmin is pinned, the distributed snapshot's xip array grows,
> the snapshot handed out by the k-th ANALYZE commit is O(k) in size,
> and with the total spill files grow O(k^2), even though the slot's
> own database is completely idle.
>
> Note that a decoding session is connected to a single database and can only
> reads that database's catalogs plus the shared catalogs, so changes on other
> databases' catalog doesn't need a snapshot rebuild and distribution. The
> attached patch exploits this: in DecodeCommit(), if the committing
> transaction's dbId differs from the slot's database and none of the
> commit record's invalidation messages touches a shared catalog, it skips
> both the snapshot rebuild and the distribution.
>
> Correctness relies on that, every write to a shared catalog
> produces invalidation messages, SHAREDINVALSNAPSHOT_ID or
> SHAREDINVALRELCACHE_ID.
>
> Repro/measurement (script attached): one test_decoding slot on db1;
> 3000 tables created in db2; a long transaction open in db2; then a
> single ANALYZE over all of db2's tables, which commits each table
> separately (logical_decoding_work_mem = 64kB); finally decode the WAL
> with the slot. spill_bytes from pg_stat_replication_slots for db1's
> slot:
> master ~19 MB
> patched 0
>
> So the slot of an idle database is completely unaffected by another
> database's ANALYZE traffic or other operations that can generate
> invalidation messages, while decoding of its own database's
> catalog changes is unchanged.
>
> Any feedback is appreciated.

Hi,

I started to review the patch. And some have doubts.
In the following code:
+ if (parsed->dbId != ctx->slot->data.database &&
+ !InvalidationsTouchSharedCatalog(parsed->nmsgs, parsed->msgs))
+ distribute = false;
We are checking parsed->msgs for any invalidation message for shared catalog.

But when we are actually distributing the invalidation, we are using:
```
if (txn->xid != xid)
{
uint32 ninvalidations;
SharedInvalidationMessage *msgs = NULL;

ninvalidations = ReorderBufferGetInvalidations(builder->reorder,
xid, &msgs);

if (ninvalidations > 0)
{
Assert(msgs != NULL);

ReorderBufferAddDistributedInvalidations(builder->reorder,
txn->xid, lsn, ninvalidations, msgs);
}
}
```

Can the invalidations returned by ReorderBufferGetInvalidations()
differ from parsed->msgs?
More specifically, can parsed->msgs miss an invalidation related to a
shared catalog, while that invalidation is present in the
ReorderBuffer? If so, InvalidationsTouchSharedCatalog() could return
false and distribute would be set to false, causing us to skip
distribution of an invalidation that is actually needed.

I actually tried a case on HEAD. Suppose S1 and S2 are two psql
sessions. And suppose we execute it in this order:
Session S1 and S2 are running on separate databases db1 and db2. And
logical replication is setup in db1.
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?

Thanks,
Shlok Kyal

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message chee.wooson 2026-09-03 09:27:37 [PATCH v1 0/1] Avoid carrying self lock-only xmax to updated tuple
Previous Message shveta malik 2026-09-03 09:06:33 Re: Fix resource leak in FindConflictTuple() retry path