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 <mich...@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
