From: | Justin Pryzby <pryzby(at)telsasoft(dot)com> |
---|---|
To: | Julien Rouhaud <rjuju123(at)gmail(dot)com> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: expose parallel leader in CSV and log_line_prefix |
Date: | 2020-03-18 21:25:11 |
Message-ID: | 20200318212511.GN26184@telsasoft.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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/
> > https://www.postgresql.org/message-id/flat/CAOBaU_Yy5bt0vTPZ2_LUM6cUcGeqmYNoJ8-Rgto+c2+w3defYA(at)mail(dot)gmail(dot)com
> > b025f32e0b Add leader_pid to pg_stat_activity
>
> FTR this is a followup of https://www.postgresql.org/message-id/20200315095728.GA26184%40telsasoft.com
Yes - but I wasn't going to draw attention to the first patch, in which I did
something needlessly complicated and indirect. :)
> + case 'k':
> + if (MyBackendType != B_BG_WORKER)
> + ; /* Do nothing */
>
>
> Isn't the test inverted? Also a bgworker could run parallel queries through
> SPI I think, should we really ignore bgworkers?
I don't think it's reversed, but I think I see your point: the patch is
supposed to be showing the leader's own PID for the leader itself. So I think
that can just be removed.
> + else if (!MyProc->lockGroupLeader)
> + ; /* Do nothing */
>
> There should be a test that MyProc isn't NULL.
Yes, done.
> + else if (padding != 0)
> + appendStringInfo(buf, "%*d", padding, MyProc->lockGroupLeader->pid);
> + else
> + appendStringInfo(buf, "%d", MyProc->lockGroupLeader->pid);
> + break;
>
> I think that if padding was asked we should append spaces rather than doing
> nothing.
Done
It logs like:
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 [5537 5537]LOG: statement: SET log_temp_files=0;
SET
2020-03-15 21:20:47.289 CDT [5537 5537]LOG: statement: explain analyze SELECT a,COUNT(1) FROM t a JOIN t b USING(a) GROUP BY 1;
2020-03-15 21:20:51.253 CDT [5627 5537]LOG: temporary file: path "base/pgsql_tmp/pgsql_tmp5627.0", size 6094848
2020-03-15 21:20:51.253 CDT [5627 5537]STATEMENT: explain analyze SELECT a,COUNT(1) FROM t a JOIN t b USING(a) GROUP BY 1;
2020-03-15 21:20:51.254 CDT [5626 5537]LOG: temporary file: path "base/pgsql_tmp/pgsql_tmp5626.0", size 6103040
2020-03-15 21:20:51.254 CDT [5626 5537]STATEMENT: explain analyze SELECT a,COUNT(1) FROM t a JOIN t b USING(a) GROUP BY 1;
2020-03-15 21:20:51.263 CDT [5537 5537]LOG: temporary file: path "base/pgsql_tmp/pgsql_tmp5537.1.sharedfileset/o15of16.p0.0", size 557056
Now, with the leader showing its own PID.
This also fixes unsafe access to lockGroupLeader->pid, same issue as in the
original v1 patch for b025f32e0b.
--
Justin
Attachment | Content-Type | Size |
---|---|---|
v3-0001-Include-the-leader-PID-in-logfile.patch | text/x-diff | 3.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2020-03-18 22:01:22 | Re: BEFORE ROW triggers for partitioned tables |
Previous Message | Julien Rouhaud | 2020-03-18 21:02:29 | Re: Collation versioning |