From: | Julien Rouhaud <rjuju123(at)gmail(dot)com> |
---|---|
To: | Justin Pryzby <pryzby(at)telsasoft(dot)com> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Daniel Gustafsson <daniel(at)yesql(dot)se>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: expose parallel leader in CSV and log_line_prefix |
Date: | 2020-07-10 15:13:26 |
Message-ID: | 20200710151326.fics6czbbvloplsk@nol |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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?
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2020-07-10 16:01:10 | Re: GSSENC'ed connection stalls while reconnection attempts. |
Previous Message | Matthieu Garrigues | 2020-07-10 15:08:20 | libpq: Request Pipelining/Batching status ? |