Re: OOM in spgist insert

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: OOM in spgist insert
Date: 2021-05-14 20:23:37
Message-ID: 2192150.1621023817@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> writes:
> This comment made me remember a patch I've had for a while, which splits
> the CHECK_FOR_INTERRUPTS() definition in two -- one of them is
> INTERRUPTS_PENDING_CONDITION() which let us test the condition
> separately; that allows the lock we hold to be released prior to
> actually processing the interrupts.

I've now pushed that macro change ...

> The btree code modified was found to be an actual problem in production
> when a btree is corrupted in such a way that vacuum would get an
> infinite loop. I don't remember the exact details but I think we saw
> vacuum running for a couple of weeks, and had to restart the server in
> order to terminate it (since it wouldn't respond to signals).

... but I think this bit still needs work, if we still want it at all.
The problem is that it seems to believe that ProcessInterrupts is
guaranteed not to return, which is far from the truth. Maybe it was
true once, but we've grown a lot of accretions on it that will just
clear InterruptPending and return. I see that the "return false"
leads directly to an "Assert(false)", which seems unhelpful.

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2021-05-14 20:53:16 Re: Some other CLOBBER_CACHE_ALWAYS culprits
Previous Message Bruce Momjian 2021-05-14 20:01:32 Re: PG 14 release notes, first draft