Re: [HACKERS] Why does logical replication launcher setapplication_name? - Mailing list pgsql-hackers

On 16/04/17 22:27, Petr Jelinek wrote:
> On 16/04/17 18:47, Tom Lane wrote:
>> Craig Ringer <craig@2ndquadrant.com> writes:
>>> On 12 April 2017 at 13:34, Kuntal Ghosh <kuntalghosh.2007@gmail.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_activitywhere 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