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

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Kirill Reshke <reshkekirill(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: GIN index fast list search may become un-interruptible for long time.
Date: 2026-07-05 17:15:45
Message-ID: f7eeaba4-fa1d-4cfd-ab73-44ba393937b9@iki.fi
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: 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

In response to

Browse pgsql-hackers by date

  From Date Subject
Previous Message Rui Zhao 2026-07-05 16:09:55 Re: [PATCH] Preserve replication origin OIDs in pg_upgrade