Re: expose parallel leader in CSV and log_line_prefix

2020-08-02 Thread Michael Paquier
On Fri, Jul 31, 2020 at 10:31:13PM -0500, Justin Pryzby wrote: > Also, I was reminded by Tom's c410af098 about this comment: > > * Further note: At least on some platforms, passing %*s > rather than > * %s to appendStringInfo() is substantially slower, so many

Re: expose parallel leader in CSV and log_line_prefix

2020-07-31 Thread Justin Pryzby
On Fri, Jul 31, 2020 at 03:04:48PM -0500, Justin Pryzby wrote: > On Tue, Jul 28, 2020 at 10:10:33AM +0900, Michael Paquier wrote: > > 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. > > Revised with your

Re: expose parallel leader in CSV and log_line_prefix

2020-07-31 Thread Justin Pryzby
On Tue, Jul 28, 2020 at 10:10:33AM +0900, Michael Paquier wrote: > 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. Revised with your suggestions. -- Justin >From 72184bdb83eba6e477a12b743bb8d6bf938c5f41 Mon

Re: expose parallel leader in CSV and log_line_prefix

2020-07-27 Thread Michael Paquier
On Sun, Jul 26, 2020 at 01:54:27PM -0500, Justin Pryzby wrote: > + > + %P > + For a parallel worker, this is the Process ID of its > leader > + process. > + > + no > + Let's be a maximum simple and consistent

Re: expose parallel leader in CSV and log_line_prefix

2020-07-26 Thread Justin Pryzby
On Sun, Jul 26, 2020 at 04:42:06PM +0900, Michael Paquier wrote: > On Thu, Jul 23, 2020 at 09:52:14AM +0900, Michael Paquier wrote: > > Sounds fine to me. Thanks. > > > > Do others have any objections with this wording? > > I have used the wording suggested by Alvaro, and applied the patch >

Re: expose parallel leader in CSV and log_line_prefix

2020-07-26 Thread Michael Paquier
On Thu, Jul 23, 2020 at 09:52:14AM +0900, Michael Paquier wrote: > Sounds fine to me. Thanks. > > Do others have any objections with this wording? I have used the wording suggested by Alvaro, and applied the patch down to 13. Now let's see about the original item of this thread.. -- Michael

Re: expose parallel leader in CSV and log_line_prefix

2020-07-22 Thread Michael Paquier
On Wed, Jul 22, 2020 at 08:59:04PM -0400, Tom Lane wrote: > Is "NULL" really le mot juste here? If we're talking about text strings, > as the thread title implies (I've not read the patch), then I think you > should say "empty string", because the SQL concept of null doesn't apply. Sorry for the

Re: expose parallel leader in CSV and log_line_prefix

2020-07-22 Thread Tom Lane
Michael Paquier writes: > On Wed, Jul 22, 2020 at 11:36:05AM -0400, Alvaro Herrera wrote: >> How about we combine both. "Process ID of the parallel group leader, if >> this process is a parallel query worker. NULL if this process is a >> parallel group leader or does not participate in parallel

Re: expose parallel leader in CSV and log_line_prefix

2020-07-22 Thread Michael Paquier
On Wed, Jul 22, 2020 at 11:36:05AM -0400, Alvaro Herrera wrote: > How about we combine both. "Process ID of the parallel group leader, if > this process is a parallel query worker. NULL if this process is a > parallel group leader or does not participate in parallel query". Sounds fine to me.

Re: expose parallel leader in CSV and log_line_prefix

2020-07-22 Thread Alvaro Herrera
On 2020-Jul-21, Michael Paquier wrote: > On Mon, Jul 20, 2020 at 11:12:31PM -0500, Justin Pryzby wrote: > >> + Process ID of the parallel group leader if this process is involved > >> + in parallel query, or null. For a parallel group leader, this > >> field > >> + is NULL. >

Re: expose parallel leader in CSV and log_line_prefix

2020-07-22 Thread Julien Rouhaud
On Tue, Jul 21, 2020 at 6:33 AM Michael Paquier wrote: > > On Mon, Jul 20, 2020 at 11:12:31PM -0500, Justin Pryzby wrote: > > On Tue, Jul 21, 2020 at 12:51:45PM +0900, Michael Paquier wrote: > > The documentation could talk about either: > > > > 1) "lock group leader" - low-level, raw view of the

Re: expose parallel leader in CSV and log_line_prefix

2020-07-20 Thread Michael Paquier
On Fri, Jul 17, 2020 at 07:34:54AM +0200, Julien Rouhaud wrote: > I had the same concern and was thinking about this approach too. > Another argument is that IIUC any log emitted due to > log_min_duration_statement wouldn't see the backend as executing a > parallel query, since the workers would

Re: expose parallel leader in CSV and log_line_prefix

2020-07-20 Thread Michael Paquier
On Mon, Jul 20, 2020 at 11:12:31PM -0500, Justin Pryzby wrote: > On Tue, Jul 21, 2020 at 12:51:45PM +0900, Michael Paquier wrote: > The documentation could talk about either: > > 1) "lock group leader" - low-level, raw view of the internal data structure > (with a secondary mention that "for a

Re: expose parallel leader in CSV and log_line_prefix

2020-07-20 Thread Justin Pryzby
On Tue, Jul 21, 2020 at 12:51:45PM +0900, Michael Paquier wrote: > And I'd rather keep the simple suggestion of upthread to leave the > field as NULL for the parallel group leader with a PID match but not a > backend type check so as this could be useful for other types of > processes. The

Re: expose parallel leader in CSV and log_line_prefix

2020-07-20 Thread Michael Paquier
On Mon, Jul 20, 2020 at 06:30:48PM -0500, Justin Pryzby wrote: > This thread is about a new feature that I proposed which isn't yet committed > (logging leader_pid). But it raises a question which is immediately relevant > to pg_stat_activity.leader_pid, which is committed for v13. So feel free

Re: expose parallel leader in CSV and log_line_prefix

2020-07-20 Thread Justin Pryzby
On Fri, Jul 17, 2020 at 05:32:36PM -0500, Justin Pryzby wrote: > On Fri, Jul 17, 2020 at 05:27:21PM -0400, Alvaro Herrera wrote: > > On 2020-Jul-17, Justin Pryzby wrote: > > > Ok, but should we then consider changing pg_stat_activity for consistency > > > ? > > > Probably in v13 to avoid changing

Re: expose parallel leader in CSV and log_line_prefix

2020-07-17 Thread Justin Pryzby
On Fri, Jul 17, 2020 at 05:27:21PM -0400, Alvaro Herrera wrote: > On 2020-Jul-17, Justin Pryzby wrote: > > Ok, but should we then consider changing pg_stat_activity for consistency ? > > Probably in v13 to avoid changing it a year later. > >

Re: expose parallel leader in CSV and log_line_prefix

2020-07-17 Thread Alvaro Herrera
On 2020-Jul-17, Justin Pryzby wrote: > Ok, but should we then consider changing pg_stat_activity for consistency ? > Probably in v13 to avoid changing it a year later. > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=b025f32e0b5d7668daec9bfa957edf3599f4baa8 > > I think the

Re: expose parallel leader in CSV and log_line_prefix

2020-07-17 Thread Justin Pryzby
On Fri, Jul 17, 2020 at 11:35:40AM -0400, Alvaro Herrera wrote: > > On Fri, Jul 17, 2020 at 7:01 AM Michael Paquier wrote: > > > > > Hmm. Knowing if a leader is actually running parallel query or not > > > requires a lookup at lockGroupMembers, that itself requires a LWLock. > > > I think that

Re: expose parallel leader in CSV and log_line_prefix

2020-07-17 Thread Alvaro Herrera
> On Fri, Jul 17, 2020 at 7:01 AM Michael Paquier wrote: > > > Hmm. Knowing if a leader is actually running parallel query or not > > requires a lookup at lockGroupMembers, that itself requires a LWLock. > > I think that it would be better to not require that. So what if > > instead we logged

Re: expose parallel leader in CSV and log_line_prefix

2020-07-16 Thread Julien Rouhaud
Hi, On Fri, Jul 17, 2020 at 7:01 AM Michael Paquier wrote: > > On Thu, Jul 16, 2020 at 10:55:45PM -0400, Alvaro Herrera wrote: > > Oh, ugh, I don't like that part much. If you run connections through a > > connection pooler, it's going to be everywhere. Let's put it there only > > if the

Re: expose parallel leader in CSV and log_line_prefix

2020-07-16 Thread Michael Paquier
On Thu, Jul 16, 2020 at 10:55:45PM -0400, Alvaro Herrera wrote: > Oh, ugh, I don't like that part much. If you run connections through a > connection pooler, it's going to be everywhere. Let's put it there only > if the connection *is* running a parallel query, without being too > stressed about

Re: expose parallel leader in CSV and log_line_prefix

2020-07-16 Thread Alvaro Herrera
On 2020-Jul-17, Michael Paquier wrote: > Please note that this choice comes from BecomeLockGroupLeader(), where > a leader registers itself in lockGroupLeader, and remains set as such > as long as the process is alive so we would always get a value for a > process once it got involved in parallel

Re: expose parallel leader in CSV and log_line_prefix

2020-07-16 Thread Michael Paquier
On Fri, Jul 10, 2020 at 01:16:40PM -0400, Alvaro Herrera wrote: > On 2020-Jul-10, Justin Pryzby wrote: >> That's what's done. >> >> + Process ID of the parallel group leader if this process >> was >> + at some point involved in parallel query, otherwise null. For >> a

Re: expose parallel leader in CSV and log_line_prefix

2020-07-10 Thread Alvaro Herrera
On 2020-Jul-10, Justin Pryzby wrote: > On Fri, Jul 10, 2020 at 12:45:29PM -0400, Alvaro Herrera wrote: > > I think it's overly verbose; all non-parallel backends are going to get > > their own PID twice, and I'm not sure this is going to be great to > > parse. I think it would be more sensible

Re: expose parallel leader in CSV and log_line_prefix

2020-07-10 Thread Justin Pryzby
On Fri, Jul 10, 2020 at 12:45:29PM -0400, Alvaro Herrera wrote: > On 2020-Mar-18, Justin Pryzby wrote: > > > On Sun, Mar 15, 2020 at 12:49:33PM +0100, Julien Rouhaud wrote: > > > template1=# SET log_temp_files=0; explain analyze SELECT a,COUNT(1) FROM t > > a JOIN t b USING(a) GROUP BY 1; > >

Re: expose parallel leader in CSV and log_line_prefix

2020-07-10 Thread Alvaro Herrera
On 2020-Mar-18, Justin Pryzby wrote: > On Sun, Mar 15, 2020 at 12:49:33PM +0100, Julien Rouhaud wrote: > template1=# SET log_temp_files=0; explain analyze SELECT a,COUNT(1) FROM t a > JOIN t b USING(a) GROUP BY 1; > 2020-03-15 21:20:47.288 CDT [55375537]LOG: statement: SET >

Re: expose parallel leader in CSV and log_line_prefix

2020-07-10 Thread Julien Rouhaud
On Fri, Jul 10, 2020 at 11:11:15AM -0500, Justin Pryzby wrote: > 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

Re: expose parallel leader in CSV and log_line_prefix

2020-07-10 Thread Justin Pryzby
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

Re: expose parallel leader in CSV and log_line_prefix

2020-07-10 Thread Julien Rouhaud
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

Re: expose parallel leader in CSV and log_line_prefix

2020-07-09 Thread Justin Pryzby
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

Re: expose parallel leader in CSV and log_line_prefix

2020-07-09 Thread Michael Paquier
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

Re: expose parallel leader in CSV and log_line_prefix

2020-07-09 Thread Julien Rouhaud
On Thu, Jul 9, 2020 at 1:48 PM Daniel Gustafsson wrote: > > > On 18 Mar 2020, at 22:25, Justin Pryzby wrote: > > > This also fixes unsafe access to lockGroupLeader->pid, same issue as in the > > original v1 patch for b025f32e0b. > > Julian, having been involved in the other threads around this

Re: expose parallel leader in CSV and log_line_prefix

2020-07-09 Thread Daniel Gustafsson
> On 18 Mar 2020, at 22:25, Justin Pryzby wrote: > This also fixes unsafe access to lockGroupLeader->pid, same issue as in the > original v1 patch for b025f32e0b. Julian, having been involved in the other threads around this topic, do you have time to review this latest version during the

Re: expose parallel leader in CSV and log_line_prefix

2020-03-18 Thread Justin Pryzby
On Sun, Mar 15, 2020 at 12:49:33PM +0100, Julien Rouhaud wrote: > On Sun, Mar 15, 2020 at 06:18:31AM -0500, Justin Pryzby wrote: > > See also: > > https://commitfest.postgresql.org/27/2390/ > >

Re: expose parallel leader in CSV and log_line_prefix

2020-03-15 Thread Julien Rouhaud
On Sun, Mar 15, 2020 at 06:18:31AM -0500, Justin Pryzby wrote: > See also: > https://commitfest.postgresql.org/27/2390/ > https://www.postgresql.org/message-id/flat/caobau_yy5bt0vtpz2_lum6cucgeqmynoj8-rgto+c2+w3de...@mail.gmail.com > b025f32e0b Add leader_pid to pg_stat_activity FTR this is a

expose parallel leader in CSV and log_line_prefix

2020-03-15 Thread Justin Pryzby
See also: https://commitfest.postgresql.org/27/2390/ https://www.postgresql.org/message-id/flat/caobau_yy5bt0vtpz2_lum6cucgeqmynoj8-rgto+c2+w3de...@mail.gmail.com b025f32e0b Add leader_pid to pg_stat_activity -- Justin >From 5268e89fb32fbb639cb39796729dfe0dfcf14705 Mon Sep 17 00:00:00 2001 From: