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?