Re: GIN index fast list search may become un-interruptible for long time. - Mailing list pgsql-hackers

From Heikki Linnakangas
Subject Re: GIN index fast list search may become un-interruptible for long time.
Date
Msg-id f7eeaba4-fa1d-4cfd-ab73-44ba393937b9@iki.fi
Whole thread
In response to GIN index fast list search may become un-interruptible for long time.  (Kirill Reshke <reshkekirill@gmail.com>)
List pgsql-hackers
On 04/07/2026 20:38, Kirill Reshke wrote:
> Recently I have found a case in one of our production clusters where
> the query does not respond to pg_*_backend(pid).
> I have found a small synthetic reproducer for this.
> 
> ...
> 
> The thing is, we hold a fast-list buffer lock while scanning it, which
> can take time. But backends are uninterruptible (via postgres cancel)
> when holding a lock on buffer. This strikes me as not too good
> concurrency design. For now, I post v1-0001 where we simply check if
> the interrupt condition is pending, and if it is, we simply unlock our
> fast list buffer and do CFI(), which should cancel the query at this
> point. This is simply to show-case where we are stuck and also for
> triggering problems which I fix in 0002.
> 
> I understand that my v1 is uncommittable because of possible
> performance implications. I also think this
> INTERRUPTS_PENDING_CONDITION is too clumsy. But right now I am looking
> for a back-patchable fix, so maybe v1 is bad for master, but for
> back-branches maybe fine.

I'm not too worried about the performance of CHECK_FOR_INTERRUPTS(), 
although did you actually try to measure it? You could also e.g. only 
check for interrupts every 100 items or something like that, if that's a 
problem.

However, the locking is broken:

> --- a/src/backend/access/gin/ginget.c
> +++ b/src/backend/access/gin/ginget.c
> @@ -1666,6 +1666,13 @@ collectMatchesForHeapRow(IndexScanDesc scan, pendingPosition *pos)
>                              StopHigh = pos->lastOffset,
>                              StopMiddle;
>  
> +                if (INTERRUPTS_PENDING_CONDITION()) {
> +
> +                    UnlockReleaseBuffer(pos->pendingBuffer);
> +
> +                    CHECK_FOR_INTERRUPTS();
> +                }
> +
>                  /* If already matched on earlier page, do no extra work */
>                  if (key->entryRes[j])
>                      continue;

CHECK_FOR_INTERRUPTS() is not guaranteed to bail out, even if 
INTERRUPTS_PENDING_CONDITION() returns true. If it returns normally, we 
have already released the buffer and continue without holding the lock 
anymore.


The reason that the loop takes so long is that we look up every key item 
individually on the pending list. In your example, the key has about 
900000 items. With such a large number of key items, it would be much 
faster to sort the key items, and do a "merge join" of the key items and 
the item on the page. That's a bigger patch though, and not 
backpatchable. But aside from the interrupt issue, it would make it much 
faster.

- Heikki



pgsql-hackers by date:

Previous
From: Pavel Stehule
Date:
Subject: Re: POC: PLpgSQL FOREACH IN JSON ARRAY
Next
From: Heikki Linnakangas
Date:
Subject: Split fd.c into multiple source files