On Thu, Nov 17, 2022 at 11:15 PM Andres Freund <and...@anarazel.de> wrote: > > On 2022-11-17 10:44:18 +0530, Amit Kapila wrote: > > On Wed, Nov 16, 2022 at 11:56 PM Andres Freund <and...@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 <and...@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 <and...@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. >
Okay, updated the patch accordingly. -- With Regards, Amit Kapila.
v3-0001-Add-additional-checks-while-creating-the-initial-.patch
Description: Binary data
