| From: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
|---|---|
| To: | Peter Geoghegan <pg(at)bowt(dot)ie> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, scott(at)scottray(dot)io |
| Subject: | Re: Snapshot export on a standby corrupts hint bits on subxact overflow |
| Date: | 2026-07-28 06:07:34 |
| Message-ID: | amhHJiZyFA9s70Uq@bdtpg |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Sun, Jul 26, 2026 at 10:17:21PM -0400, Peter Geoghegan wrote:
> I decided to reinvestigate the problem today, with help from Claude
> code. I found a bug that exactly matches the known symptoms. Attached
> patch 0001 has a reproducer + draft bug fix. This is likely a bug in
> 2017 commit 6c2003f8. There's also a second patch 0002 that fixes
> another bug found along the way (though that's much less serious than
> the one that 0001 deals with).
Thanks for having looked at it! The first problem is also something I worked
on in the past without success.
> The test case in 0001 shows a scenario where pg_export_snapshot on a
> standby hands out a snapshot that claims no transaction is running,
> which is wrong. A backend that imports it writes wrong hint bits,
> which are then seen by every other session on that standby --
> including sessions that never touched the exported snapshot.
> User-visible symptoms include rows reappearing after deletion, rows
> vanishing after insertion, and duplicate entries in unique indexes
> (all symptoms that I've personally seen in the wild).
The fix in 0001 makes sense to me.
Some comments:
=== 1
+ if (snapshot->subxcnt + nchildren > GetMaxSnapshotSubxidCount())
+ ereport(ERROR,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
Yes, this check is needed here.
+ for (int32 i = 0; i < snapshot->subxcnt; i++)
+ appendStringInfo(&buf, "sxp:%u\n", snapshot->subxip[i]);
+ for (int32 i = 0; i < nchildren; i++)
+ appendStringInfo(&buf, "sxp:%u\n", children[i]);
We count every existing subxip entry and every committed child without filtering
against snapshot->xmax. A committed child created after the imported snapshot
was taken can have an XID >= snapshot->xmax. I think that such an entry is
unnecessary for visibility, since XidInMVCCSnapshot() classifies every XID >= xmax
as still in progress before looking at subxip.
It can be observed by applying the attached post-xmax.txt on top of 0001, which
produces:
# BDT snapshot saved for post-promotion re-export 00000007-00000002-1: sof=1
# BDT re-exported snapshot 00000000-00000004-1: sof=1; xmax=781; sxp=[698, 763, 764, 765, 766, 767, 768, 769, 770, 771, 772, 773, 774, 775, 776, 777, 778, 782]; sxp >= xmax=[782]
We can see that xmax=781, and that 782 has been exported.
That's not a visibility issue, however, those entries count toward GetMaxSnapshotSubxidCount()
and could therefore cause an avoidable PROGRAM_LIMIT_EXCEEDED.
=== 2
> 0002 is a separate bug of the same general nature.
+ if (cur->takenDuringRecovery)
+ {
+ nxip = cur->subxcnt;
+ xip = cur->subxip;
+ }
+ else
SnapshotData.subxip permits entries at or above xmax, whereas pg_snapshot.xip
requires every entry to satisfy xmin <= xip[i] < xmax.
So, I think filtering is appropriate in both places:
1/ In ExportSnapshot(), do not include recovery subxip entries and committed
child XIDs at or above xmax when counting and serializing them, so unnecessary
entries do not consume the limited recovery subxip capacity.
2/ In pg_current_snapshot(), do not include source XIDs outside [xmin, xmax),
so that it enforces the rule regardless of how the source snapshot was produced.
=== 3
0002 can now expose subtransaction IDs, so I think some comments:
"Note that only top-level transaction IDs are exposed to user sessions"
"Note that only top-transaction XIDs are included in the snapshot"
and the docs for pg_current_snapshot() and xip_list need updates to mention the
recovery specific exception.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
| Attachment | Content-Type | Size |
|---|---|---|
| post-xmax.txt | text/plain | 1.8 KB |
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | shveta malik | 2026-07-28 05:41:16 | Re: A new C function `get_partition_root`. |