From: Julien Rouhaud <rjuju123@gmail.com>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Daniel Gustafsson <daniel@yesql.se>
Cc: pgsql-hackers@postgresql.org
Subject: Re: expose parallel leader in CSV and log_line_prefix
Date: Fri, 10 Jul 2020 17:13:26 +0200
Message-ID: <20200710151326.fics6czbbvloplsk@nol> (raw)
In-Reply-To: <20200710022023.GE26220@telsasoft.com>
References: <20200315111831.GA21492@telsasoft.com>
<20200315114933.udjdx5wcgwkjnw45@nol>
<20200318212511.GN26184@telsasoft.com>
<8EC64544-2EAA-48B1-8B69-85158CA3EAE9@yesql.se>
<CAOBaU_a4=GXdqtXsU-2_o3oekEx3fsbCu-8ZB21oJm+CtfxNkA@mail.gmail.com>
<20200710020940.GJ11153@paquier.xyz>
<20200710022023.GE26220@telsasoft.com>
On Thu, Jul 09, 2020 at 09:20:23PM -0500, Justin Pryzby wrote:
> On Fri, Jul 10, 2020 at 11:09:40AM +0900, Michael Paquier wrote:
> > On Thu, Jul 09, 2020 at 01:53:39PM +0200, Julien Rouhaud wrote:
> > > Sure! I've been quite busy with internal work duties recently but
> > > I'll review this patch shortly. Thanks for the reminder!
> >
> > Hmm. In which cases would it be useful to have this information in
> > the logs knowing that pg_stat_activity lets us know the link between
> > both the leader and its workers?
>
> PSA is an instantaneous view whereas the logs are a record. That's important
> for shortlived processes (like background workers) or in the case of an ERROR
> or later crash.
>
> Right now, the logs fail to include that information, which is deficient. Half
> the utility is in showing *that* the log is for a parallel worker, which is
> otherwise not apparent.
Yes, I agree that this is a nice thing to have and another smell step toward
parallel query monitoring.
About the patch:
+ case 'k':
+ if (MyProc)
+ {
+ PGPROC *leader = MyProc->lockGroupLeader;
+ if (leader == NULL)
+ /* padding only */
+ appendStringInfoSpaces(buf,
+ padding > 0 ? padding : -padding);
+ else if (padding != 0)
+ appendStringInfo(buf, "%*d", padding, leader->pid);
+ else
+ appendStringInfo(buf, "%d", leader->pid);
+ }
+ break;
There's a thinko in the padding handling. It should be dones whether MyProc
and/or lockGroupLeader is NULL or not, and only if padding was asked, like it's
done for case 'd' for instance.
Also, the '%k' escape sounds a bit random. Is there any reason why we don't
use any uppercase character for log_line_prefix? %P could be a better
alternative, otherwise maybe %g, as GroupLeader/Gather?
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: rjuju123@gmail.com, pryzby@telsasoft.com, michael@paquier.xyz, daniel@yesql.se
Subject: Re: expose parallel leader in CSV and log_line_prefix
In-Reply-To: <20200710151326.fics6czbbvloplsk@nol>
* 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