pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: Fix contrib/auto_explain to not cause problems in parallel worke
7+ messages / 2 participants
[nested] [flat]
* pgsql: Fix contrib/auto_explain to not cause problems in parallel worke
@ 2019-06-03 22:06 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 7+ messages in thread
From: Tom Lane @ 2019-06-03 22:06 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix contrib/auto_explain to not cause problems in parallel workers.
A parallel worker process should not be making any decisions of its
own about whether to auto-explain. If the parent session process
passed down flags asking for instrumentation data, do that, otherwise
not. Trying to enable instrumentation anyway leads to bugs like the
"could not find key N in shm TOC" failure reported in bug #15821
from Christian Hofstaedtler.
We can implement this cheaply by piggybacking on the existing logic
for not doing anything when we've chosen not to sample a statement.
While at it, clean up some tin-eared coding related to the sampling
feature, including an off-by-one error that meant that asking for 1.0
sampling rate didn't actually result in sampling every statement.
Although the specific case reported here only manifested in >= v11,
I believe that related misbehaviors can be demonstrated in any version
that has parallel query; and the off-by-one error is certainly there
back to 9.6 where that feature was added. So back-patch to 9.6.
Discussion: https://postgr.es/m/15821-5eb422e980594075@postgresql.org
Branch
------
REL_11_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/57e85fa2cb7e7a99306be4b62c6a547532b7e849
Modified Files
--------------
contrib/auto_explain/auto_explain.c | 43 ++++++++++++++++++++++++-------------
1 file changed, 28 insertions(+), 15 deletions(-)
^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix contrib/auto_explain to not cause problems in parallel worke
@ 2019-06-03 22:06 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 7+ messages in thread
From: Tom Lane @ 2019-06-03 22:06 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix contrib/auto_explain to not cause problems in parallel workers.
A parallel worker process should not be making any decisions of its
own about whether to auto-explain. If the parent session process
passed down flags asking for instrumentation data, do that, otherwise
not. Trying to enable instrumentation anyway leads to bugs like the
"could not find key N in shm TOC" failure reported in bug #15821
from Christian Hofstaedtler.
We can implement this cheaply by piggybacking on the existing logic
for not doing anything when we've chosen not to sample a statement.
While at it, clean up some tin-eared coding related to the sampling
feature, including an off-by-one error that meant that asking for 1.0
sampling rate didn't actually result in sampling every statement.
Although the specific case reported here only manifested in >= v11,
I believe that related misbehaviors can be demonstrated in any version
that has parallel query; and the off-by-one error is certainly there
back to 9.6 where that feature was added. So back-patch to 9.6.
Discussion: https://postgr.es/m/15821-5eb422e980594075@postgresql.org
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/2cd4e835701e15f5e32dd8bde488f8669ddf7ae8
Modified Files
--------------
contrib/auto_explain/auto_explain.c | 43 ++++++++++++++++++++++++-------------
1 file changed, 28 insertions(+), 15 deletions(-)
^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix contrib/auto_explain to not cause problems in parallel worke
@ 2019-06-03 22:06 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 7+ messages in thread
From: Tom Lane @ 2019-06-03 22:06 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix contrib/auto_explain to not cause problems in parallel workers.
A parallel worker process should not be making any decisions of its
own about whether to auto-explain. If the parent session process
passed down flags asking for instrumentation data, do that, otherwise
not. Trying to enable instrumentation anyway leads to bugs like the
"could not find key N in shm TOC" failure reported in bug #15821
from Christian Hofstaedtler.
We can implement this cheaply by piggybacking on the existing logic
for not doing anything when we've chosen not to sample a statement.
While at it, clean up some tin-eared coding related to the sampling
feature, including an off-by-one error that meant that asking for 1.0
sampling rate didn't actually result in sampling every statement.
Although the specific case reported here only manifested in >= v11,
I believe that related misbehaviors can be demonstrated in any version
that has parallel query; and the off-by-one error is certainly there
back to 9.6 where that feature was added. So back-patch to 9.6.
Discussion: https://postgr.es/m/15821-5eb422e980594075@postgresql.org
Branch
------
REL_10_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/ba38967d7567342bb547d8aafa8e9bab24396df4
Modified Files
--------------
contrib/auto_explain/auto_explain.c | 43 ++++++++++++++++++++++++-------------
1 file changed, 28 insertions(+), 15 deletions(-)
^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix contrib/auto_explain to not cause problems in parallel worke
@ 2019-06-03 22:06 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 7+ messages in thread
From: Tom Lane @ 2019-06-03 22:06 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix contrib/auto_explain to not cause problems in parallel workers.
A parallel worker process should not be making any decisions of its
own about whether to auto-explain. If the parent session process
passed down flags asking for instrumentation data, do that, otherwise
not. Trying to enable instrumentation anyway leads to bugs like the
"could not find key N in shm TOC" failure reported in bug #15821
from Christian Hofstaedtler.
We can implement this cheaply by piggybacking on the existing logic
for not doing anything when we've chosen not to sample a statement.
While at it, clean up some tin-eared coding related to the sampling
feature, including an off-by-one error that meant that asking for 1.0
sampling rate didn't actually result in sampling every statement.
Although the specific case reported here only manifested in >= v11,
I believe that related misbehaviors can be demonstrated in any version
that has parallel query; and the off-by-one error is certainly there
back to 9.6 where that feature was added. So back-patch to 9.6.
Discussion: https://postgr.es/m/15821-5eb422e980594075@postgresql.org
Branch
------
REL9_6_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/efa121ae6a86f493cd40d0e22dfce5e75ec1f94b
Modified Files
--------------
contrib/auto_explain/auto_explain.c | 43 ++++++++++++++++++++++++-------------
1 file changed, 28 insertions(+), 15 deletions(-)
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pgsql: Fix contrib/auto_explain to not cause problems in parallel worke
@ 2019-06-04 01:13 Michael Paquier <michael@paquier.xyz>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 7+ messages in thread
From: Michael Paquier @ 2019-06-04 01:13 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org
Hi Tom,
On Mon, Jun 03, 2019 at 10:06:21PM +0000, Tom Lane wrote:
> Fix contrib/auto_explain to not cause problems in parallel workers.
>
> A parallel worker process should not be making any decisions of its
> own about whether to auto-explain. If the parent session process
> passed down flags asking for instrumentation data, do that, otherwise
> not. Trying to enable instrumentation anyway leads to bugs like the
> "could not find key N in shm TOC" failure reported in bug #15821
> from Christian Hofstaedtler.
This has broken the builds on Windows for 9.6 and 10:
https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=woodlouse&dt=2019-06-04%2000%3A42%...
Here is the failure:
auto_explain.obj : error LNK2001: unresolved external symbol
ParallelWorkerNumber [C:\buildfarm\buildenv\REL_10_STABLE\pgsql.build\auto_explain.vcxproj]
This gets included as a dependency because of the call to
IsParallelWorker() you have added in this commit, and I think that
this can be fixed by adding PGDLLIMPORT to the variable in
parallel.h. Would you fix it or should I?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../20190604011354.GD1529@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pgsql: Fix contrib/auto_explain to not cause problems in parallel worke
@ 2019-06-04 01:16 Tom Lane <tgl@sss.pgh.pa.us>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 7+ messages in thread
From: Tom Lane @ 2019-06-04 01:16 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-committers@lists.postgresql.org
Michael Paquier <michael@paquier.xyz> writes:
> On Mon, Jun 03, 2019 at 10:06:21PM +0000, Tom Lane wrote:
>> Fix contrib/auto_explain to not cause problems in parallel workers.
> This has broken the builds on Windows for 9.6 and 10:
> https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=woodlouse&dt=2019-06-04%2000%3A42%...
Ooops.
> This gets included as a dependency because of the call to
> IsParallelWorker() you have added in this commit, and I think that
> this can be fixed by adding PGDLLIMPORT to the variable in
> parallel.h. Would you fix it or should I?
Yeah, I see we already did that in later branches. I'll go fix it,
thanks for the heads-up!
regards, tom lane
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pgsql: Fix contrib/auto_explain to not cause problems in parallel worke
@ 2019-06-04 01:29 Michael Paquier <michael@paquier.xyz>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 7+ messages in thread
From: Michael Paquier @ 2019-06-04 01:29 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org
On Mon, Jun 03, 2019 at 09:16:56PM -0400, Tom Lane wrote:
> Yeah, I see we already did that in later branches. I'll go fix it,
> thanks for the heads-up!
Thanks.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../20190604012919.GF1529@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2019-06-04 01:29 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-06-03 22:06 pgsql: Fix contrib/auto_explain to not cause problems in parallel worke Tom Lane <tgl@sss.pgh.pa.us>
2019-06-03 22:06 pgsql: Fix contrib/auto_explain to not cause problems in parallel worke Tom Lane <tgl@sss.pgh.pa.us>
2019-06-03 22:06 pgsql: Fix contrib/auto_explain to not cause problems in parallel worke Tom Lane <tgl@sss.pgh.pa.us>
2019-06-04 01:13 ` Michael Paquier <michael@paquier.xyz>
2019-06-04 01:16 ` Tom Lane <tgl@sss.pgh.pa.us>
2019-06-04 01:29 ` Michael Paquier <michael@paquier.xyz>
2019-06-03 22:06 pgsql: Fix contrib/auto_explain to not cause problems in parallel worke Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox