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.

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

best regards,
Florian Pflug


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to