On Tue, Mar 21, 2017 at 10:37 PM, Kuntal Ghosh <kuntalghosh.2...@gmail.com> wrote: > On Tue, Mar 21, 2017 at 10:52 AM, Michael Paquier > <michael.paqu...@gmail.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
