On Wed, Dec 30, 2009 at 6:43 PM, Andres Freund <and...@anarazel.de> wrote: > > ----- Ursprüngliche Mitteilung ----- >> On Wed, Dec 30, 2009 at 2:06 PM, Andres Freund <and...@anarazel.de> wrote: >> > On Wednesday 30 December 2009 01:13:01 Simon Riggs wrote: >> > > On Tue, 2009-12-29 at 11:13 -0500, Tom Lane wrote: >> > > > Andres Freund <and...@anarazel.de> writes: >> > > > > On Tuesday 29 December 2009 16:22:54 Tom Lane wrote: >> > > > > > This seems like a fairly bad idea. One of the intended use-cases >> > > > > > is >> > > > > > to be able to manually "kill -INT" a misbehaving backend. Assuming >> > > > > > that there will be valid info about the signal in shared memory >> > > > > > will >> > > > > > break that. >> > > > > >> > > > > Well. That already is the case now. MyProc->recoveryConflictMode is >> > > > > checked to recognize what kind of conflict is being resolved... >> > > > >> > > > In that case, HS has already broken it, and we need to fix it not make >> > > > it worse. >> > > > >> > > > My humble opinion is that SIGINT should not be overloaded with multiple >> > > > meanings. We already have a multiplexed signal mechanism, which is >> > > > what >> > > > should be used for any additional signal reasons HS may need to >> > > > introduce. >> > > >> > > It's a revelation to me, but yes, I see it now and agree. >> > > >> > > I'm looking at Fujii-san's multiplexing patch from Jul 31 to rewrite >> > > this code using that mechanism. It sounds like it's a neat fit and it >> > > should get around the bug report from Kris also if it all works. >> > Hm. I just read a bit of that multiplexing facility (out of a different >> > reason) >> > and I have some doubt about it being used unmodified for canceling >> > backends: >> > >> > procsignal.c: >> > /* >> > * Note: Since there's no locking, it's possible that the target >> > * process detaches from shared memory and exits right after this >> > * test, before we set the flag and send signal. And the signal slot >> > * might even be recycled by a new process, so it's remotely possible >> > * that we set a flag for a wrong process. That's OK, all the signals >> > * are such that no harm is done if they're mistakenly fired. >> > */ >> > procsignal.h: >> > ... >> > * Also, because of race conditions, it's important that all the signals be >> > * defined so that no harm is done if a process mistakenly receives one. >> > */ >> > >> > When cancelling a backend that behaviour could be a bit annoying ;-) >> > >> > I guess locking procarray during sending the signal should be enough? >> >> I think the idea is that you define the behavior of the signal to be >> "look at this other piece of state to see whether you should cancel >> yourself" rather than just "cancel yourself". Then if a signal is >> delivered by mistake, it's no big deal - you just look at the other >> piece of state and decide that you don't need to do anything. > I dont see an easy way to pass enough information right now. You cant > regenerate enough of it in the to be killed backend as most of the relevant > information is only available in the startup process. > Inventing yet another segment in shm just for this seems overcomplicated to > me. > Thats why I suggested locking the procarray for this - without having looked > at the code that should prevent a backend slot from beimg reused.
Yeah, I understand, but I have a feeling that the code doesn't do it that way right now for a reason. Someone who understands this better than I should comment, but I'm thinking you would likely need to lock the ProcArray in CheckProcSignal as well, and I'm thinking that can't be safely done from within a signal handler. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
