Thank you Robert for committing the patch. commit fc70a4b0df38bda6a13941f1581f25fbb643c7f3
I've changed the status to Committed. On Mon, Mar 27, 2017 at 6:09 AM, Michael Paquier <michael.paqu...@gmail.com> wrote: > On Sat, Mar 25, 2017 at 5:26 PM, Kuntal Ghosh > <kuntalghosh.2...@gmail.com> wrote: >> On Fri, Mar 24, 2017 at 9:23 PM, Robert Haas <robertmh...@gmail.com> wrote: >>> I think this is still not good. The places where pgstat_bestart() has >>> been added are not even correct. For example, the call added to >>> BackgroundWriterMain() occurs after the section that does >>> error-recovery, so it would get repeated every time the background >>> writer recovers from an error. There are similar problems elsewhere. >>> Furthermore, although in theory there's an idea here that we're making >>> it no longer the responsibility of InitPostgres() to call >>> pgstat_bestart(), the patch as proposed only removes one of the two >>> calls, so we really don't even have a consistent practice. I think >>> it's better to go with the idea of having InitPostgres() be >>> responsible for calling this for regular backends, and >>> AuxiliaryProcessMain() for auxiliary backends. That involves >>> substantially fewer calls to pgstat_bestart() and they are spread >>> across only two functions, which IMHO makes fewer bugs of omission a >>> lot less likely. >> >> Agreed. Calling it from InitPostgres() and AuxiliaryProcessMain() >> seems correct because of the following two reasons as you've mentioned >> up in the thread: >> 1. security-filtering should be left to some higher-level facility >> that can make policy decisions rather than being hard-coded in the >> individual modules. >> 2. makes fewer bugs of omission a lot less likely. > > Okay, fine for me. > >>> - I modified the code to tolerate a NULL return from >>> AuxiliaryPidGetProc(). I am pretty sure that without that there's a >>> race condition that could lead to a crash if somebody tried to call >>> this function just as an auxiliary process was terminating. >> >> Wow. Haven't thought of that. If it's called after >> AuxiliaryProcKill(), a crash is evident. > > This one is a good catch. > -- > Michael -- Thanks & Regards, Kuntal Ghosh 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