On Mon, Sep 18, 2017 at 10:00 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Amit Kapila <amit.kapil...@gmail.com> writes: >> Attached patch fixes these problems. > > Hmm, this patch adds a kill(notify_pid) after one call to > ForgetBackgroundWorker, but the postmaster has several more such calls. > Shouldn't they all notify the notify_pid? Should we move that > functionality into ForgetBackgroundWorker itself, so we can't forget > it again? >
Among other places, we already notify during ReportBackgroundWorkerExit(). However, it seems to me that all other places except where this patch has added a call to notify doesn't need such a call. The other places like in DetermineSleepTime and ResetBackgroundWorkerCrashTimes is called for a crashed worker which I don't think requires notification to the backend as that backend itself would have restarted. The other place where we call ForgetBackgroundWorker is in maybe_start_bgworkers when rw_terminate is set to true which again seems to be either the case of worker crash or when someone has explicitly asked to terminate the worker in which case we already send a notification. I think we need to notify the backend on start, stop and failure to start a worker. OTOH, if it is harmless to send a notification even after the worker is crashed, then we can just move that functionality into ForgetBackgroundWorker itself as that will simplify the code and infact that is the first thing that occurred to me as well, but I haven't done that way as I was not sure if we want to send notification in all kind of cases. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
