Re: Assertion failure in SnapBuildInitialSnapshot()

Hi,

On 2022-11-17 10:44:18 +0530, Amit Kapila wrote:
> On Wed, Nov 16, 2022 at 11:56 PM Andres Freund <andres@anarazel.de> wrote:
> > On 2022-11-16 14:22:01 +0530, Amit Kapila wrote:
> > > On Wed, Nov 16, 2022 at 7:30 AM Andres Freund <andres@anarazel.de> wrote:
> > > > On 2022-11-15 16:20:00 +0530, Amit Kapila wrote:
> > > > > On Tue, Nov 15, 2022 at 8:08 AM Andres Freund <andres@anarazel.de> wrote:
> > I don't think that'd catch a catalog snapshot. But perhaps the better answer
> > for the catalog snapshot is to just invalidate it explicitly. The user doesn't
> > have control over the catalog snapshot being taken, and it's not too hard to
> > imagine the walsender code triggering one somewhere.
> >
> > So maybe we should add something like:
> >
> > InvalidateCatalogSnapshot(); /* about to overwrite MyProc->xmin */
> >
> 
> The comment "/* about to overwrite MyProc->xmin */" is unclear to me.
> We already have a check (/* so we don't overwrite the existing value
> */
> if (TransactionIdIsValid(MyProc->xmin))) in SnapBuildInitialSnapshot()
> which ensures that we don't overwrite MyProc->xmin, so the above
> comment seems contradictory to me.

The point is that catalog snapshots could easily end up setting MyProc->xmin,
even though the caller hasn't done anything wrong. So the
InvalidateCatalogSnapshot() would avoid erroring out in a number of scenarios.

Greetings,

Andres Freund