Re: exposing wait events for non-backends (was: Tracking wait event for latches)

From: Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Craig Ringer <craig(at)2ndquadrant(dot)com>, Kevin Grittner <kgrittn(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: exposing wait events for non-backends (was: Tracking wait event for latches)
Date: 2017-03-27 09:14:12
Message-ID: CAGz5QC+vrcbd3ay34Q6p2gZ17DPyk=U_L6sCH1-y=NsVSnX9qQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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(dot)paquier(at)gmail(dot)com> wrote:
> On Sat, Mar 25, 2017 at 5:26 PM, Kuntal Ghosh
> <kuntalghosh(dot)2007(at)gmail(dot)com> wrote:
>> On Fri, Mar 24, 2017 at 9:23 PM, Robert Haas <robertmhaas(at)gmail(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavan Deolasee 2017-03-27 09:24:34 Re: Patch: Write Amplification Reduction Method (WARM)
Previous Message Dilip Kumar 2017-03-27 09:02:03 Re: Parallel bitmap heap scan