Hi,
On 2022-07-12 17:13:28 +0300, Yura Sokolov wrote:
> В Сб, 19/02/2022 в 13:31 -0800, Andres Freund пишет:
> > Hi,
> >
> > On 2022-02-19 10:00:02 -0800, Andres Freund wrote:
> > > See backtrace below [1]. The same problem does *not* exist when starting to
> > > use the same temp schema in a new session (which drops the old contents
> > > first), which kind of explains why we've not previously noticed this.
> > >
> > > But even so, I'm surprised we haven't noticed this before.
> >
> > Ah, there's a reason for that. In many cases we'll have a catalog snapshot
> > registered, which is enough for init_toast_snapshot(). But in Miles' example,
> > the object dropped just prior ends with catalog invalidations.
> >
> > Proposed bugfix and test attached.
> >
> > I think it's ok to backpatch the test. There might be a slight change in
> > output due to 618c16707a6d6e8f5c83ede2092975e4670201ad not being backpatched,
> > but that's OK I think.
> >
> >
> > I think it is dangerous that we return a cached catalog snapshot for things
> > like GetOldestSnapshot() unless they're also registered or active - we can't
> > rely on catalog snapshots to be present. Indeed, if we didn't, this bug would
> > have been found before, as some added assertions confirm.
> >
> > I don't think we can just ignore the catalog snapshot though, it can be
> > registered in the future, so it actually is the oldest snapshot. But at least
> > we should assert that there's some snapshot registered/active. In the attached
> > patch I've added HaveRegisteredOrActiveSnapshot() and used that in
> > init_toast_snapshot().
>
> Reading your message and HaveRegisteredOrActiveSnapshot's body, I can't get its logic:
> - if it is dangerous to have CatalogSnapshot alone in RegisteredSnapshots,
> then why we return 'false' if RegisteredSnapshots is NOT singular?
IIRC that was a bug that since was fixed. Did you check the current
definition?
Greetings,
Andres Freund
