Re: Issue with bgworker, SPI and pgstat_report_stat

On 07/11/2016 06:53 AM, Craig Ringer wrote:
> On 11 July 2016 at 11:49, Michael Paquier <michael(dot)paquier(at)gmail(dot)com
> <mailto:michael(dot)paquier(at)gmail(dot)com>> wrote:
>
> On Mon, Jul 11, 2016 at 3:36 AM, Julien Rouhaud
> <julien(dot)rouhaud(at)dalibo(dot)com <mailto:julien(dot)rouhaud(at)dalibo(dot)com>> wrote:
> > I'm not opposed, but in this case we should also provide a proper
> > documentation of all the required actions to mimick normal backends.
>
> I'd rather just add a note like "Have a look at PostgresMain if you
> want to imitate a backend able to run queries!" instead of listing all
> the actions doable. If low-level things are added or removed in the
> future in PostgresMain, it is very likely that the documentation will
> not be updated at the same time, killing its purpose at the end.
>
>
> That sounds like a bug breeding ground. Especially with extensions whose
> bgworkers operate across Pg versions, extensions that get updated
> without re-checking PostgresMain and don't notice some new housekeeping
> task, etc.
>
> Rather than encouraging every extension author to copy and paste random
> chunks of PostgresMain, probably incorrectly, I really think factoring
> the required logic out into something reusable by bgworkers is going to
> be the way to go.
>

I'm not sure I agree with this. Clearly, the fact that worker_spi does
not invoke pgstat_report_stat() is a bug, but as Michael points out, we
don't have much insight into what is happening in bgworkers.

Following the changes in PostgresMain() - particularly if your bgworker
needs to support multiple versions - is difficult, sure. But well, if
you decided to implement a bgworker operating at such low level, you
voluntarily accepts that responsibility.

That does not mean we can't make that easier. For example, what about
extending the bgworker API with

(a) a set of flags (similar to bgw_flags) identifying which maintenance
tasks the bgworker requests, and

(b) a BackgroundWorkerCleanup() function the bgworker might place at a
suitable place, invoking all the requested maintenance tasks?

Sure, this may only help for bgworkers that do stuff fairly close to
regular backends, but maybe that's enough.

In any case, I think adding the pgstat_report_stat() into worker_spi
seems like a reasonable (and backpatchable) fix.

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services