pgsql: Fix inadequacies in recently added wait events

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

Fix inadequacies in recently added wait events

In commit 9915de6c1cb2, we introduced a new wait point for replication
slots and incorrectly labelled it as wait event PG_WAIT_LOCK. That's
wrong, so invent an appropriate new wait event instead, and document it
properly.

While at it, fix numerous other problems in the vicinity:
- two different walreceiver wait events were being mixed up in a single
wait event (which wasn't documented either); split it out so that they
can be distinguished, and document the new events properly.

- ParallelBitmapPopulate was documented but didn't exist.

- ParallelBitmapScan was not documented (I think this should be called
"ParallelBitmapScanInit" instead.)

- Logical replication wait events weren't documented

- various symbols had been added in dartboard order in various places.
Put them in alphabetical order instead, as was originally intended.

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]/messages/by-id/CAEepm=3da6aJYxiUk03pAKt1t7eGWAnn9CajC773Ct1rDPO-Hw@mail.gmail.com[2]/messages/by-id/CAEepm=0B7yM9MZSviq1d-hnt4KoaRVeJvSfAyVfykNV-pVDqug@mail.gmail.com.

[1]: /messages/by-id/CAEepm=3da6aJYxiUk03pAKt1t7eGWAnn9CajC773Ct1rDPO-Hw@mail.gmail.com
[2]: /messages/by-id/CAEepm=0B7yM9MZSviq1d-hnt4KoaRVeJvSfAyVfykNV-pVDqug@mail.gmail.com

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers