Re: Why does logical replication launcher set application_name?

On 16/04/17 18:47, Tom Lane wrote:
> Craig Ringer <craig(at)2ndquadrant(dot)com> writes:
>> On 12 April 2017 at 13:34, Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com> wrote:
>>> For backend_type=background worker, application_name shows the name of
>>> the background worker (BackgroundWorker->bgw_name). I think we need
>>> some way to distinguish among different background workers. But,
>>> application_name may not be the correct field to show the information.
>
>> It's better than (ab)using 'query' IMO.
>
>> I'd rather an abbreviated entry to address Tom's concerns about
>> format. 'lrlaunch' or whatever.
>
> Basically the problem I've got with the LR launcher is that it looks
> utterly unlike any other background process in pg_stat_activity.
> Leaving out all-null columns to make my point:
>
> regression=# select pid,usesysid,usename,application_name,backend_start,wait_event_type,wait_event,backend_type from pg_stat_activity where application_name != 'psql';
> pid | usesysid | usename | application_name | backend_start | wait_event_type | wait_event | backend_type
> -------+----------+----------+------------------------------+-------------------------------+-----------------+---------------------+---------------------
> 25416 | | | | 2017-04-16 12:32:46.987076-04 | Activity | AutoVacuumMain | autovacuum launcher
> 25418 | 10 | postgres | logical replication launcher | 2017-04-16 12:32:46.988859-04 | Activity | LogicalLauncherMain | background worker
> 25414 | | | | 2017-04-16 12:32:46.986745-04 | Activity | BgWriterHibernate | background writer
> 25413 | | | | 2017-04-16 12:32:46.986885-04 | Activity | CheckpointerMain | checkpointer
> 25415 | | | | 2017-04-16 12:32:46.9871-04 | Activity | WalWriterMain | walwriter
> (5 rows)
>
> Why has it got non-null entries for usesysid and usename, never mind
> application_name? Why does it not follow the well-established convention
> that backend_type is what identifies background processes?
>
> I'm sure the answer to those questions is "it's an implementation artifact
> from using the generic bgworker infrastructure", but that does not make it
> look any less like sloppy, half-finished work.
>
> If it is a limitation of the bgworker infrastructure that we can't make
> the LR processes look more like the other kinds of built-in processes,
> then I think we need to fix that limitation. And I further assert that
> we need to do it for v10, because once we ship v10 people will adjust
> their tools for this bogus output, and we'll face complaints about
> backwards compatibility if we fix it later.
>

It's indeed how bgworker infrastructure is reporting it. That being
said, since LR processes are in-core, we can add exception for them in
pgstat_bestart() so that they are reported more like other builtin
processes. We could also try to add api for bgworker processes to change
how they are reported so that any future workers (and all the external
workers) can be reported properly as well, but that seems better fit for
v11 at this point since it would be good if we had some discussion for
how that should look like.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services