Re: Proposal for Signal Detection Refactoring - Mailing list pgsql-hackers

From Chris Travers
Subject Re: Proposal for Signal Detection Refactoring
Date
Msg-id CAN-RpxB_MMUJhbOocDVOYsLZV4729xHpZkB6e+tqF7LLNCE1_Q@mail.gmail.com
Whole thread Raw
In response to Re: Proposal for Signal Detection Refactoring  (Michael Paquier <michael@paquier.xyz>)
Responses Re: Proposal for Signal Detection Refactoring
List pgsql-hackers
First, thanks for taking the time to write this.  Its very helpful.  Additional thoughts inline.

On Mon, Sep 24, 2018 at 2:12 AM Michael Paquier <michael@paquier.xyz> wrote:
On Fri, Sep 21, 2018 at 12:35:46PM +0200, Chris Travers wrote:
> I understand how lock levels don't fit a simple hierarchy but at least
> when it comes to what is going to be aborted on a signal, I am having
> trouble understanding the problem here.

It may be possible to come with a clear hierarchy with the current
interruption types in place.  Still I am not sure that the definition
you put behind is completely correct, and I think that we need to
question as well the value of putting such restrictions for future
interruption types because they would need to fit into it.

The future-safety issue is a really good one and it's one reason I kept the infinite loop patch as semantically consistent with the API as I could at the cost of some complexity.

I have another area where I think a patch would be more valuable anyway in terms of refactoring.
 
  That's quite
a heavy constraint to live with.  There is such logic with wal_level for
example, which is something I am not completely happy with either...
But this one is a story for another time, and another thread.

 From a cleanup perspective a concentric circles approach seems like it is correct to me (which would correspond to a hierarchy of interrupts) but I can see that assuming that all pending interrupts would be checked solely for cleanup reasons might be a bad assumption on my part.

Regarding your patch, it seems to me that it does not improve
readability as I mentioned up-thread because you lose sight of what can
be interrupted in a given code path, which is what the current code
shows actually nicely.

So I guess there are two fundamental questions here.

1.  Do we want to move away from checking global flags like this directly?  I think we do because it makes future changes possibly harder and more complex since there is no encapsulation of logic.  But I don't see a point in putting effort into that without consensus.

There could be value in refactoring things so as all the *Pending flags
of miscadmin.h get stored into one single volatile sig_atomic_t which
uses bit-wise markers, as that's at least 4 bytes because that's stored
as an int for most platforms and can be performed as an atomic operation
safely across signals (If my memory is right;) ).  And this leaves a lot
of room for future flags.

Yeah I will look into this.

Thanks again for taking the time to go over the concerns in detail.  It really helps.

Best Wishes,
Chris Travers
--
Michael


--
Best Regards,
Chris Travers
Head of Database

Tel: +49 162 9037 210 | Skype: einhverfr | www.adjust.com 
Saarbrücker Straße 37a, 10405 Berlin

pgsql-hackers by date:

Previous
From: Noah Misch
Date:
Subject: Re: Something fishy happening on frogmouth
Next
From: Andrey Borodin
Date:
Subject: Re: Global snapshots