From: Michael Paquier <michael@paquier.xyz>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Julien Rouhaud <rjuju123@gmail.com>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: expose parallel leader in CSV and log_line_prefix
Date: Tue, 28 Jul 2020 10:10:33 +0900
Message-ID: <20200728011033.GA28700@paquier.xyz> (raw)
In-Reply-To: <20200726185427.GI4286@telsasoft.com>
References: <20200721043325.GC17300@paquier.xyz>
<20200722153605.GA961@alvherre.pgsql>
<20200723005214.GD26630@paquier.xyz>
<20200726074206.GB4351@paquier.xyz>
<20200726185427.GI4286@telsasoft.com>
On Sun, Jul 26, 2020 at 01:54:27PM -0500, Justin Pryzby wrote:
> + <row>
> + <entry><literal>%P</literal></entry>
> + <entry>For a parallel worker, this is the Process ID of its leader
> + process.
> + </entry>
> + <entry>no</entry>
> + </row>
Let's be a maximum simple and consistent with surrounding descriptions
here and what we have for pg_stat_activity, say:
"Process ID of the parallel group leader, if this process is a
parallel query worker."
> + case 'P':
> + if (MyProc)
> + {
> + PGPROC *leader = MyProc->lockGroupLeader;
> + if (leader == NULL || leader->pid == MyProcPid)
> + /* padding only */
> + appendStringInfoSpaces(buf,
> + padding > 0 ? padding : -padding);
> + else if (padding != 0)
> + appendStringInfo(buf, "%*d", padding, leader->pid);
> + else
> + appendStringInfo(buf, "%d", leader->pid);
It seems to me we should document here that the check on MyProcPid
ensures that this only prints the leader PID only for parallel workers
and discards the leader.
> + appendStringInfoChar(&buf, ',');
> +
> + /* leader PID */
> + if (MyProc)
> + {
> + PGPROC *leader = MyProc->lockGroupLeader;
> + if (leader && leader->pid != MyProcPid)
> + appendStringInfo(&buf, "%d", leader->pid);
> + }
> +
Same here.
Except for those nits, I have tested the patch and things behave as we
want (including padding and docs), so this looks good to me.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../20200728011033.GA28700@paquier.xyz/2-signature.asc)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: michael@paquier.xyz, pryzby@telsasoft.com, alvherre@2ndquadrant.com, rjuju123@gmail.com, tomas.vondra@2ndquadrant.com
Subject: Re: expose parallel leader in CSV and log_line_prefix
In-Reply-To: <20200728011033.GA28700@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
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