Re: Why does logical replication launcher set application_name?

From: Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Craig Ringer <craig(at)2ndquadrant(dot)com>
Cc: Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Why does logical replication launcher set application_name?
Date: 2017-04-18 16:13:23
Message-ID: af5dc6ce-3f4a-5ba7-2a1b-c7f9afded1ee@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 16/04/17 22:27, Petr Jelinek wrote:
> 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.
>

Hmm so I took a look at code today with intention to implement this, but
it does not seem as straightforward as I'd hoped due to pgstat_bestart()
being called quite early in startup.

We can definitely easily detect that the bgworker is internal one by
library_name equals 'postgres' so we can easily remove the usesysid and
usename based on that. But that does not solve the issue of identifying
the processes in pg_stat_activity as logical replication laucher/worker.
I wonder if it would be okay to set backend_type to bgw_name for
internal workers and just leave the external ones as it is (or solve
them in v11 with some proper API) as we can control the length of name
there (it will still be longer than the values for other things but
maybe not too much).

Does that sound reasonable enough for v10?

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2017-04-18 16:13:26 Re: Interval for launching the table sync worker
Previous Message Jesper Pedersen 2017-04-18 16:08:46 dtrace probes