On 19 Oct 2011, at 17:54, Florian Pflug wrote: > On Oct19, 2011, at 17:47 , Greg Jaskiewicz wrote: >> On 15 Oct 2011, at 11:31, Florian Pflug wrote: >>> >>> Ok, here's a first cut. >> >> So I looked at the patch, and first thing that pops out, >> is lack of the volatile keyword before the ClientConnectionLostPending >> variable is defined. Is that done on purpose ? Is that on purpose ? > > That's on purpose. volatile is only necessary for variables which are either > accessed from within signal handlers or which live in shared memory. Neither > is true for ClientConnectionLostPending, so non-volatile should be fine. Ok, cool. I'm aware of the reasons behind volatile, just noticed that some other flags used in similar way are marked as such. At the end of the day, this is just a hint to the compiler anyway.
> >> Otherwise the patch itself looks ok. >> I haven't tested the code, just reviewed the patch itself. And it obviously >> needs testing, should be easy to follow your original problem description. > > Yeah, further testing is on my todo list. The interesting case is probably > what happens if the connection is dropped while there's already a > cancellation request pending. And also the other way around - a cancellation > request arriving after we've already discovered that the connection is gone. > >> Btw, I just tried to do it through commitfest.postgresql.org , but before I >> get my head around on how to add myself to the reviewer list there - I >> thought I'll just send this response here. > > You just need to click "Edit Patch" and put your name into the Reviewer > field. You do need a postgres community account to edit patches, but the > signup process is quite quick and painless AFAIR. Ok, clicking 'edit patch' sounds a bit big. Probably better would be, to just be able to click on some sort of "I'm in" button/checkbox type of thing. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
