From: Justin Pryzby <pryzby@telsasoft.com>
To: Julien Rouhaud <rjuju123@gmail.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 11:11:15 -0500
Message-ID: <20200710161115.GK26220@telsasoft.com> (raw)
In-Reply-To: <20200710151326.fics6czbbvloplsk@nol>
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>
<20200710151326.fics6czbbvloplsk@nol>
On Fri, Jul 10, 2020 at 05:13:26PM +0200, Julien Rouhaud wrote:
> 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?
Thanks for looking. %P is a good idea - it's consistent with ps and pkill and
probably other %commands. I also amended the docs.
--
Justin
Attachments:
[text/x-diff] v2-0001-Include-the-leader-PID-in-logfile.patch (4.0K, ../20200710161115.GK26220@telsasoft.com/2-v2-0001-Include-the-leader-PID-in-logfile.patch)
download | inline diff:
From 68a892d0023e915cc48aea1bb77eae5deabcecec Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Fri, 13 Mar 2020 22:03:06 -0500
Subject: [PATCH v2] Include the leader PID in logfile
See also: b025f32e0b, which adds the leader PID to pg_stat_activity
---
doc/src/sgml/config.sgml | 11 ++++++-
src/backend/utils/error/elog.c | 30 +++++++++++++++++++
src/backend/utils/misc/postgresql.conf.sample | 1 +
3 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgmlindex b353c61683..e60403d6a5 100644--- a/doc/src/sgml/config.sgml+++ b/doc/src/sgml/config.sgml@@ -6611,6 +6611,14 @@ local0.* /var/log/postgresql
<entry>Process ID</entry>
<entry>no</entry>
</row>
+ <row>+ <entry><literal>%P</literal></entry>+ <entry>Process ID of the parallel group leader if this process was+ at some point involved in parallel query, otherwise null. For a+ parallel group leader itself, this field is set to its own process+ ID.</entry>+ <entry>no</entry>+ </row>
<row>
<entry><literal>%t</literal></entry>
<entry>Time stamp without milliseconds</entry>
@@ -6943,7 +6951,7 @@ log_line_prefix = '%m [%p] %q%u@%d/%a '
character count of the error position therein,
location of the error in the PostgreSQL source code
(if <varname>log_error_verbosity</varname> is set to <literal>verbose</literal>),
- application name, and backend type.+ application name, backend type, and leader PID.
Here is a sample table definition for storing CSV-format log output:
<programlisting>
@@ -6973,6 +6981,7 @@ CREATE TABLE postgres_log
location text,
application_name text,
backend_type text,
+ leader_pid integer,
PRIMARY KEY (session_id, session_line_num)
);
</programlisting>
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.cindex e4b717c79a..dea098faf1 100644--- a/src/backend/utils/error/elog.c+++ b/src/backend/utils/error/elog.c@@ -77,6 +77,7 @@
#include "postmaster/syslogger.h"
#include "storage/ipc.h"
#include "storage/proc.h"
+#include "storage/procarray.h"
#include "tcop/tcopprot.h"
#include "utils/guc.h"
#include "utils/memutils.h"
@@ -2448,6 +2449,25 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
else
appendStringInfo(buf, "%d", MyProcPid);
break;
++ case 'P':+ 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);+ }+ else if (padding != 0)+ appendStringInfoSpaces(buf,+ padding > 0 ? padding : -padding);+ break;+
case 'l':
if (padding != 0)
appendStringInfo(buf, "%*ld", padding, log_line_number);
@@ -2836,6 +2856,16 @@ write_csvlog(ErrorData *edata)
else
appendCSVLiteral(&buf, GetBackendTypeDesc(MyBackendType));
+ appendStringInfoChar(&buf, ',');++ /* leader PID */+ if (MyProc)+ {+ PGPROC *leader = MyProc->lockGroupLeader;+ if (leader)+ appendStringInfo(&buf, "%d", leader->pid);+ }+
appendStringInfoChar(&buf, '\n');
/* If in the syslogger process, try to write messages direct to file */
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sampleindex e430e33c7b..48a50fb394 100644--- a/src/backend/utils/misc/postgresql.conf.sample+++ b/src/backend/utils/misc/postgresql.conf.sample@@ -533,6 +533,7 @@
# %h = remote host
# %b = backend type
# %p = process ID
+ # %P = leader PID
# %t = timestamp without milliseconds
# %m = timestamp with milliseconds
# %n = timestamp with milliseconds (as a Unix epoch)
--
2.17.0
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: pryzby@telsasoft.com, rjuju123@gmail.com, michael@paquier.xyz, daniel@yesql.se
Subject: Re: expose parallel leader in CSV and log_line_prefix
In-Reply-To: <20200710161115.GK26220@telsasoft.com>
* 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