Re: Is RecoveryConflictInterrupt() entirely safe in a signal handler?

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Is RecoveryConflictInterrupt() entirely safe in a signal handler?
Date: 2022-06-22 01:04:05
Message-ID: YrJqhSezuvgE63kn@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jun 21, 2022 at 11:02:57PM +1200, Thomas Munro wrote:
> On Tue, Jun 21, 2022 at 7:44 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>> The extra business with QueryCancelHoldoffCount and DoingCommandRead
>> is the only addition for the snapshot, lock and tablespace conflict
>> handling part. I don't see why a reason why that could be wrong on a
>> close lookup. Anyway, why don't you check QueryCancelPending on top
>> of QueryCancelHoldoffCount?
>
> The idea of this patch is to make ProcessRecoveryConflictInterrupt()
> throw its own ERROR, instead of setting QueryCancelPending (as an
> earlier version of the patch did). It still has to respect
> QueryCancelHoldoffCount, though, to avoid emitting an ERROR at bad
> times for the fe/be protocol.

Yeah, I was reading through v3 and my brain questioned the
inconsistency, but I can see that v2 already did that and I have also
looked at it. Anyway, my concern here is that the code becomes more
dependent on the ordering of ProcessRecoveryConflictInterrupt() and
the code path checking for QueryCancelPending in ProcessInterrupts().
With the patch, we should always have QueryCancelPending set to false,
as long as there are no QueryCancelHoldoffCount. Perhaps an extra
assertion for QueryCancelPending could be added at the beginning of
ProcessRecoveryConflictInterrupts(), in combination of the one already
present for InterruptHoldoffCount. I agree that's a minor point,
though.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2022-06-22 01:41:51 Re: [BUG] Panic due to incorrect missingContrecPtr after promotion
Previous Message Tom Lane 2022-06-22 00:48:12 Re: gcc -ftabstop option