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 > <[email protected]> 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
