Snapshot export on a standby corrupts hint bits on subxact overflow

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

Hi,

On Tue, Jul 28, 2026 at 06:07:34AM +0000, Bertrand Drouvot wrote:

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.

Following our off-list discussion, PFA v2 that addresses the comments above.
It adds xmax filtering, updates the documentation, and adds a few tests.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Wed, Jul 29, 2026 at 5:36 AM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

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.

That is a valid issue, but I wonder if it's worth including in a
back-patchable fix. Is the special case worth the added risk?

Attached v3 simplifies 0001, partly by leaving that part out entirely.
It also simplifies the logic by always writing "sof:%u" and "sxcnt:%d"
to the temp file -- the idea is to make ImportSnapshot import any
subxacts it finds in the file (while still sanitizing the inputs).

The nchildren-won't-fit issue is extremely narrow in practice. The
worst that can happen is that the user sees an "invalid snapshot data
in file..." error on import. But that can only happen when:

1. A snapshot is taken on a standby.
2. The xact holding that snapshot outlives promotion.
3. The xact then writes, acquiring an XID. It accumulates enough
committed subxacts that they, plus the snapshot's own subxip entries,
exceed GetMaxSnapshotSubxidCount().
4. It then calls pg_export_snapshot().

When someone goes to import the snapshot exported in step 4, they see
the error. In practice, GetMaxSnapshotSubxidCount will fit about 15k
XIDs with max_connections=200. So I think that this will just never
happen.

Note also that it need not be the original xact that acquired the
snapshot that holds onto it in step 2. You could export + import a
snapshot during recovery, and then have the importing xact continue
after promotion -- at which point the snapshot is exported + imported
a second time. That's probably manageable in practice, but it makes me
nervous. Especially because there is evidently plenty of potential for
somebody to get something wrong in this area (we learned of three bugs
here in the past 6 weeks or so).

Maybe we could improve the error message, but I want the committed
solution to be as simple as possible.

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.

I'm not treating this one as a priority, so I haven't worked on it.

I'm focused on committing 0001 in the next few days, since it's a bug
that has caused users real harm.

--
Peter Geoghegan

Hi,

On Mon, Aug 24, 2026 at 07:07:07PM -0400, Peter Geoghegan wrote:

On Wed, Jul 29, 2026 at 5:36 AM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

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.

That is a valid issue, but I wonder if it's worth including in a
back-patchable fix. Is the special case worth the added risk?

Yeah, probably not. What about adding an XXX here:

+       /*
+        * Ignore the subxid array if it has overflowed, unless the snapshot was
+        * taken during recovery - in that case, top-level XIDs are in subxip as
+        * well, and we mustn't lose them.  CopySnapshot() and SerializeSnapshot()
+        * make the same exception.
+        */

Like:

"
* XXX: After promotion, an imported recovery snapshot can have subxip
* entries and committed children at or above xmax. These entries cannot
* affect visibility, but can make sxcnt exceed
* GetMaxSnapshotSubxidCount(), causing ImportSnapshot() to reject a
* snapshot we exported. Filtering entries outside [xmin, xmax) would avoid that.
"

so that we don't forget about it?

Attached v3 simplifies 0001, partly by leaving that part out entirely.

Thanks for the new version! Yeah, it looks simpler, let's keep it that way.

It also simplifies the logic by always writing "sof:%u" and "sxcnt:%d"
to the temp file -- the idea is to make ImportSnapshot import any
subxacts it finds in the file (while still sanitizing the inputs).

Good idea! That makes sense to me. The format change is safe to backpatch too,
since exported snapshot files are removed at startup.

Maybe we could improve the error message, but I want the committed
solution to be as simple as possible.

That makes sense. I'm not sure we should modify the error message in this
commit, let's keep the patch focus on fixing the bug?

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.

I'm not treating this one as a priority, so I haven't worked on it.

I'm focused on committing 0001 in the next few days, since it's a bug
that has caused users real harm.

Sounds good!

I looked at v3 and LGTM.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Pushed.

On Tue, Aug 25, 2026 at 5:32 AM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

On Mon, Aug 24, 2026 at 07:07:07PM -0400, Peter Geoghegan wrote:

On Wed, Jul 29, 2026 at 5:36 AM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

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.

That is a valid issue, but I wonder if it's worth including in a
back-patchable fix. Is the special case worth the added risk?

Yeah, probably not. What about adding an XXX here:

In the committed version, we raise a specific ERROR when this happens.
I think it's unlikely that any user will ever see this ERROR, but it's
better to have it and not need it.

Thanks
--
Peter Geoghegan

Hi Peter,

On 26 Aug 2026, Peter Geoghegan wrote:

Pushed.

While investigating foreign-key violations after failover, I found
tuples with HEAP_XMIN_INVALID set despite their xmin being committed.
pg_visibility reported them, but verify_heapam() did not.

I posted amcheck checks for this damage [0]/messages/by-id/9B06CC00-4F0A-423F-94E4-E3AC1F8C40DF@yandex-team.ru, including the xmax case.
Would it be worth backpatching the first patch in that series, to help
users find existing damage after installing new minors?

Thank you!

Best regards, Andrey Borodin.

[0]: /messages/by-id/9B06CC00-4F0A-423F-94E4-E3AC1F8C40DF@yandex-team.ru