On Fri, Sep 30, 2022 at 04:51:20PM -0400, Tom Lane wrote:
> Jaime Casanova writes:
> > Just to confirm I saw this on RC1
>
> What test case are you using?
>
Hi,
Currently the way I have to reproduce it is:
- install the regression database
- drop all tables but:
hash_i4_heap,
On Sat, Oct 1, 2022 at 9:35 AM Jeff Davis wrote:
> Doesn't that deal with the case you brought up directly? heap_delete()
> can't get the exclusive lock until VACUUM releases its cleanup lock, at
> which point all-visible will be set. Then heap_delete() should notice
> in line 2509 and then pin
On Fri, 2022-09-30 at 21:23 -0700, Peter Geoghegan wrote:
> 2490 page = BufferGetPage(buffer);
> 2491
> 2492 /*
> 2493 * Before locking the buffer, pin the visibility map page if
> it appears to
> 2494 * be necessary. Since we haven't got the lock yet, someone
> else might be
>
On Fri, Sep 30, 2022 at 10:13 PM Tom Lane wrote:
> How so? AFAICS these are exactly the same oversight, ie failure
> to deal with the all-visible bit getting set partway through the
> operation. You've explained how that can happen.
I thought that there might have been something protective
Peter Geoghegan writes:
> I think that the heap_delete() issue is probably in all PG versions.
Yeah, that's what I'm afraid of ...
> I am not aware of any reason why we should need the heap_update()
> fixes to be backpatched any further.
How so? AFAICS these are exactly the same oversight, ie
On Fri, Sep 30, 2022 at 9:38 PM Tom Lane wrote:
> I'm too tired to think this through completely clearly, but this
> sounds right, and what it seems to imply is that this race condition
> exists in all PG versions.
I think that the heap_delete() issue is probably in all PG versions.
> Which
Peter Geoghegan writes:
> ... nothing stops heap_delete() from getting a pin
> on a heap page that happens to have already been cleanup locked by
> another session running VACUUM. The same session could then
> (correctly) observe that the page does not have PD_ALL_VISIBLE set --
> not yet. VACUUM
On Fri, Sep 30, 2022 at 6:29 PM Peter Geoghegan wrote:
> I talked to Robins about this privately. I was wrong; there isn't a
> simple or boring explanation.
I think that I figured it out. With or without bugfix commit 163b0993,
we do these steps early in heap_delete() (this is 13 code as of
On Fri, Sep 30, 2022 at 5:09 PM Peter Geoghegan wrote:
> In any case we cannot really treat the information that we have about
> that as a bug report -- not as things stand. Why should the question
> of whether or not we ever set a page PD_ALL_VISIBLE without a cleanup
> lock on v13 be a mystery
On Fri, Sep 30, 2022 at 4:52 PM Tom Lane wrote:> >
I would be more confident here were it not for the recent
> > heap_delete() issue reported by one of my AWS colleagues (and fixed by
> > another, Jeff Davis). See commit 163b0993 if you missed it before now.
>
> Hmm, okay, though that's really a
Peter Geoghegan writes:
> On Fri, Sep 30, 2022 at 2:56 PM Tom Lane wrote:
>> Could be, because we haven't seen field reports of this in v14 yet.
> I would be more confident here were it not for the recent
> heap_delete() issue reported by one of my AWS colleagues (and fixed by
> another, Jeff
Avoid improbable PANIC during heap_update, redux.
Commit 34f581c39 intended to ensure that RelationGetBufferForTuple
would acquire a visibility-map page pin in case the otherBuffer's
all-visible bit had become set since we last had lock on that page.
But I missed a case: when we're extending the
Avoid improbable PANIC during heap_update, redux.
Commit 34f581c39 intended to ensure that RelationGetBufferForTuple
would acquire a visibility-map page pin in case the otherBuffer's
all-visible bit had become set since we last had lock on that page.
But I missed a case: when we're extending the
Avoid improbable PANIC during heap_update, redux.
Commit 34f581c39 intended to ensure that RelationGetBufferForTuple
would acquire a visibility-map page pin in case the otherBuffer's
all-visible bit had become set since we last had lock on that page.
But I missed a case: when we're extending the
Avoid improbable PANIC during heap_update, redux.
Commit 34f581c39 intended to ensure that RelationGetBufferForTuple
would acquire a visibility-map page pin in case the otherBuffer's
all-visible bit had become set since we last had lock on that page.
But I missed a case: when we're extending the
Avoid improbable PANIC during heap_update, redux.
Commit 34f581c39 intended to ensure that RelationGetBufferForTuple
would acquire a visibility-map page pin in case the otherBuffer's
all-visible bit had become set since we last had lock on that page.
But I missed a case: when we're extending the
On Fri, Sep 30, 2022 at 2:56 PM Tom Lane wrote:
> > FWIW it seems possible that the Postgres 15 vacuumlazy.c work that
> > added lazy_scan_noprune() made this scenario more likely in practice
> > -- even compared to Postgres 14.
>
> Could be, because we haven't seen field reports of this in v14
Peter Geoghegan writes:
> On Fri, Sep 30, 2022 at 2:28 PM Tom Lane wrote:
>> Ugh ... I think I see the problem. There's still one path through
>> RelationGetBufferForTuple that fails to guarantee that it's acquired
>> a vmbuffer pin if the all-visible flag becomes set in the otherBuffer.
>
I wrote:
> Ugh ... I think I see the problem. There's still one path through
> RelationGetBufferForTuple that fails to guarantee that it's acquired
> a vmbuffer pin if the all-visible flag becomes set in the otherBuffer.
> Namely, if we're forced to extend the relation, then we deal with
> vm
On Fri, Sep 30, 2022 at 2:28 PM Tom Lane wrote:
> Jaime Casanova writes:
> > Just to confirm I saw this on RC1
>
> Ugh ... I think I see the problem. There's still one path through
> RelationGetBufferForTuple that fails to guarantee that it's acquired
> a vmbuffer pin if the all-visible flag
Jaime Casanova writes:
> Just to confirm I saw this on RC1
Ugh ... I think I see the problem. There's still one path through
RelationGetBufferForTuple that fails to guarantee that it's acquired
a vmbuffer pin if the all-visible flag becomes set in the otherBuffer.
Namely, if we're forced to
Jaime Casanova writes:
> Just to confirm I saw this on RC1
What test case are you using?
regards, tom lane
On Thu, Sep 29, 2022 at 02:55:40AM -0500, Jaime Casanova wrote:
> On Tue, Apr 13, 2021 at 04:17:39PM +, Tom Lane wrote:
> > Avoid improbable PANIC during heap_update.
> >
> > heap_update needs to clear any existing "all visible" flag on
> > the old tuple's page (and on the new page too, if
On Tue, Apr 13, 2021 at 04:17:39PM +, Tom Lane wrote:
> Avoid improbable PANIC during heap_update.
>
> heap_update needs to clear any existing "all visible" flag on
> the old tuple's page (and on the new page too, if different).
> Per coding rules, to do this it must acquire pin on the
Avoid improbable PANIC during heap_update.
heap_update needs to clear any existing "all visible" flag on
the old tuple's page (and on the new page too, if different).
Per coding rules, to do this it must acquire pin on the appropriate
visibility-map page while not holding exclusive buffer lock;
Avoid improbable PANIC during heap_update.
heap_update needs to clear any existing "all visible" flag on
the old tuple's page (and on the new page too, if different).
Per coding rules, to do this it must acquire pin on the appropriate
visibility-map page while not holding exclusive buffer lock;
Avoid improbable PANIC during heap_update.
heap_update needs to clear any existing "all visible" flag on
the old tuple's page (and on the new page too, if different).
Per coding rules, to do this it must acquire pin on the appropriate
visibility-map page while not holding exclusive buffer lock;
27 matches
Mail list logo
