Re: exposing wait events for non-backends (was: Tracking wait event for latches)

On Tue, Mar 21, 2017 at 10:37 PM, Kuntal Ghosh
<kuntalghosh(dot)2007(at)gmail(dot)com> wrote:
> On Tue, Mar 21, 2017 at 10:52 AM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
> We reserve a slot for each possible BackendId, plus one for each
> possible auxiliary process type. For a non-auxiliary process,
> BackendId is used to refer the backend status in PgBackendStatus
> array. So, a bgworker without any BackendId can't initialize its'
> entry in PgBackendStatus array. In simple terms, it will not be shown
> in pg_stat_activity. I've added some comments regarding this in
> pgstat_bestart().

- * Called from InitPostgres.
- * MyDatabaseId, session userid, and application_name must be set
- * (hence, this cannot be combined with pgstat_initialize).
+ *
+ * Apart from auxiliary processes, MyBackendId, MyDatabaseId,
+ * session userid, and application_name must be set for a
+ * backend (hence, this cannot be combined with pgstat_initialize).
That looks right.

> And, any bgworker having valid BackendId will have either a valid
> userid or BOOTSTRAP_SUPERUSERID.

So looking at the area of the code in more details, my memories have
failed me a bit. InitPostgres() is setting up MyBackendId via
SharedInvalBackendInit(), something that will never be called for
backend processes not connected to a database. The bgworker for
logical replication does not do that.

>> If you want to test this configuration, feel free to use this background worker:
>> https://github.com/michaelpq/pg_plugins/tree/master/hello_world
>> This just prints an entry to the logs every 10s, and does not connect
>> to any database. Adding a call to pgstat_bestart() in hello_main
>> triggers the assertion.
>>
> In this case, pgstat_bestart() shouldn't be called from this module as
> the spawned bgworker will have invalid BackendId. By the way, thanks
> for sharing the repo link. Found a lot of interesting things to
> explore and learn. :)

RIP to this process. Not sure if that's worth the documentation. I
imagine that people usually implement bgworkers by deleting lines in
worker_spi and keeping its structure.

>> Except for the two issues pointed out in this email, I am pretty cool
>> with this patch. Nice work.
> Thank you. :)
>
> Please find the updated patches.

Okay, switched as ready for committer. One note for the committer
though: keeping the calls of pgstat_bestart() out of
BackgroundWorkerInitializeConnection() and
BackgroundWorkerInitializeConnectionByOid() keeps users the
possibility to not have backends connected to the database show up in
pg_stat_activity. This may matter for some users (cloud deployment for
example). I am as well in favor in keeping the work of those routines
minimal, without touching at pgstat.

if (!bootstrap)
CommitTransactionCommand();
+
return;
Some useless noise here.
--
Michael