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

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(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 00:39:29
Message-ID: CAB7nPqQv=DsXnLsPULZqU+iL6rJHSFoO_CwuG46gHugWuua1EA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Craig Ringer 2017-03-27 01:31:14 Re: logical decoding of two-phase transactions
Previous Message Michael Paquier 2017-03-27 00:05:14 Re: Speedup twophase transactions