Date: Tue, 25 Aug 2026 09:32:08 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Peter Geoghegan <pg@bowt.ie>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
	Andres Freund <andres@anarazel.de>, scott@scottray.io
Subject: Re: Snapshot export on a standby corrupts hint bits on subxact
 overflow
Message-ID: <ao1hGKajEbsq2i7T@bdtpg>
References: 
 <CAH2-WzmHVeYY=pjz9x8DhhxVjXHX0pvoQ-MdiB1Tt6=o2GTiKg@mail.gmail.com>
 <amhHJiZyFA9s70Uq@bdtpg>
 <amnJhkudengxKX+Y@bdtpg>
 <CAH2-WznimsP_QS-bP3xMtRsmMGDud7GDdYAvUU4MYHQiVN_Q1Q@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAH2-WznimsP_QS-bP3xMtRsmMGDud7GDdYAvUU4MYHQiVN_Q1Q@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/ao1hGKajEbsq2i7T%40bdtpg>
Precedence: bulk

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