Thread: [HACKERS] Re: [COMMITTERS] pgsql: Fix inadequacies in recently added waitevents

[HACKERS] Re: [COMMITTERS] pgsql: Fix inadequacies in recently added waitevents

From
Alvaro Herrera
Date:
Thread moved to -hackers.

Thomas Munro wrote:
> On Wed, Aug 9, 2017 at 7:39 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

> > While at it, fix numerous other problems in the vicinity:

> All of the above seem like good candidates for a checker script in
> src/tools/check_XXX.pl, a bit like the others I've talked about
> recently [1][2].

Yeah, that's one idea, but I think it'd be better to have all these
things be generated content from a single "wait_events.txt" file, like
src/backend/utils/errcodes.txt which gives birth to a whole host of
different files.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



On Tue, Aug 8, 2017 at 11:35 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Thread moved to -hackers.
>
> Thomas Munro wrote:
>> On Wed, Aug 9, 2017 at 7:39 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
>> > While at it, fix numerous other problems in the vicinity:
>
>> All of the above seem like good candidates for a checker script in
>> src/tools/check_XXX.pl, a bit like the others I've talked about
>> recently [1][2].
>
> Yeah, that's one idea, but I think it'd be better to have all these
> things be generated content from a single "wait_events.txt" file, like
> src/backend/utils/errcodes.txt which gives birth to a whole host of
> different files.

I would prefer that. Check scripts would tend to be never run. So is
there somebody willing to work on that? Or should I as I am
responsible to increasing the number of wait events?
-- 
Michael



On Wed, Aug 9, 2017 at 6:42 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Tue, Aug 8, 2017 at 11:35 PM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:
>>> All of the above seem like good candidates for a checker script in
>>> src/tools/check_XXX.pl, a bit like the others I've talked about
>>> recently [1][2].
>>
>> Yeah, that's one idea, but I think it'd be better to have all these
>> things be generated content from a single "wait_events.txt" file, like
>> src/backend/utils/errcodes.txt which gives birth to a whole host of
>> different files.
>
> I would prefer that. Check scripts would tend to be never run. So is
> there somebody willing to work on that? Or should I as I am
> responsible to increasing the number of wait events?

Yeah, that may well be a better idea in this case.  On the other hand
it wouldn't catch multiple users of the same wait event, so both
things could be useful.

As for whether hypothetical check scripts would ever be run, I was
thinking we should stick them under some make target that developers
run all the time anyway -- perhaps "check".  Shouldn't we catch simple
mechanically detectable problems as early in the pipeline as possible?

-- 
Thomas Munro
http://www.enterprisedb.com



Thomas Munro <thomas.munro@enterprisedb.com> writes:
> As for whether hypothetical check scripts would ever be run, I was
> thinking we should stick them under some make target that developers
> run all the time anyway -- perhaps "check".  Shouldn't we catch simple
> mechanically detectable problems as early in the pipeline as possible?

Adding overhead to every developer's every test cycle doesn't sound
like a win.  Possibly a reasonable compromise would be to have some
buildfarm members running this check.
        regards, tom lane



On Wed, Aug 9, 2017 at 10:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Thomas Munro <thomas.munro@enterprisedb.com> writes:
>> As for whether hypothetical check scripts would ever be run, I was
>> thinking we should stick them under some make target that developers
>> run all the time anyway -- perhaps "check".  Shouldn't we catch simple
>> mechanically detectable problems as early in the pipeline as possible?
>
> Adding overhead to every developer's every test cycle doesn't sound
> like a win.

If it takes 100ms, nobody's gonna notice.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Aug 9, 2017 at 10:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Thomas Munro <thomas.munro@enterprisedb.com> writes:
>>> As for whether hypothetical check scripts would ever be run, I was
>>> thinking we should stick them under some make target that developers
>>> run all the time anyway -- perhaps "check".  Shouldn't we catch simple
>>> mechanically detectable problems as early in the pipeline as possible?

>> Adding overhead to every developer's every test cycle doesn't sound
>> like a win.

> If it takes 100ms, nobody's gonna notice.

I doubt running a perl script that analyzes the entire backend source
code is gonna take 100ms.
        regards, tom lane



On Wed, Aug 09, 2017 at 10:56:50AM -0400, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > On Wed, Aug 9, 2017 at 10:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Thomas Munro <thomas.munro@enterprisedb.com> writes:
> >>> As for whether hypothetical check scripts would ever be run, I
> >>> was thinking we should stick them under some make target that
> >>> developers run all the time anyway -- perhaps "check".
> >>> Shouldn't we catch simple mechanically detectable problems as
> >>> early in the pipeline as possible?
> 
> >> Adding overhead to every developer's every test cycle doesn't
> >> sound like a win.
> 
> > If it takes 100ms, nobody's gonna notice.
> 
> I doubt running a perl script that analyzes the entire backend
> source code is gonna take 100ms.

What would be a reasonable maximum amount of time for such a check to take?

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate



This thread is surprising.  If we generate the few lines of code being
in trouble, we don't need any checker script, so I don't see why we'd go
the route of the checker script instead.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> This thread is surprising.  If we generate the few lines of code being
> in trouble, we don't need any checker script, so I don't see why we'd go
> the route of the checker script instead.

I think generating whatever we can from a single authoritative file
is indeed a good idea.  But I had the impression that people also wanted
to enforce a rule about "only one use of each wait event name", which'd
require a checker script, no?  (I'm not really convinced that we need
such a rule, fwiw.)
        regards, tom lane



Tom Lane wrote:

> I think generating whatever we can from a single authoritative file
> is indeed a good idea.

Yay.

> But I had the impression that people also wanted to enforce a rule
> about "only one use of each wait event name", which'd require a
> checker script, no?  (I'm not really convinced that we need such a
> rule, fwiw.)

I'm not convinced of that, either.  Of the possible problems in the
area, that seems the lesser one.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



On Wed, Aug 9, 2017 at 9:25 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> Tom Lane wrote:
>> I think generating whatever we can from a single authoritative file
>> is indeed a good idea.
>
> Yay.

Indeed.

>> But I had the impression that people also wanted to enforce a rule
>> about "only one use of each wait event name", which'd require a
>> checker script, no?  (I'm not really convinced that we need such a
>> rule, fwiw.)
>
> I'm not convinced of that, either.  Of the possible problems in the
> area, that seems the lesser one.

With a minimal maintenance effort we can be careful enough. I think
that a comment for example in pgstat.c about the usage uniqueness
would be an adapted answer.
-- 
Michael



On Thu, Aug 10, 2017 at 2:52 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> With a minimal maintenance effort we can be careful enough. I think
> that a comment for example in pgstat.c about the usage uniqueness
> would be an adapted answer.

By the way, let's discuss that on a new thread. I'll try to come up
with a patch that can be used for a base discussion soon.
-- 
Michael