From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Justin Pryzby <pryzby(at)telsasoft(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: expose parallel leader in CSV and log_line_prefix |
Date: | 2020-07-28 01:10:33 |
Message-ID: | 20200728011033.GA28700@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2020-07-28 01:35:45 | Re: stress test for parallel workers |
Previous Message | Peter Geoghegan | 2020-07-28 00:55:16 | Re: Default setting for enable_hashagg_disk |