Re: pgstat_report_activity() and parallel CREATE INDEX (was: Parallel index creation & pg_stat_activity)

On Sun, Oct 25, 2020 at 10:52:50AM -0400, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > On Sat, Oct 17, 2020 at 08:39:35AM -0700, Peter Geoghegan wrote:
> >> I also prefer 2.
> 
> > Thanks.  Here is the pair of patches I plan to use.  The second is equivalent
> > to v0; I just added a log message.
> 
> The change in worker_spi_main seems a little weird/lazy.  I'd
> be inclined to set and clear debug_query_string each time through
> the loop, so that those operations are adjacent to the other
> status-reporting operations, as they are in initialize_worker_spi.

True.  Emitting STATEMENT message fields during ProcessConfigFile(PGC_SIGHUP)
would be wrong.

> I don't really like modifying a StringInfo while debug_query_string
> is pointing at it, either.  Yeah, you'll *probably* get away with
> that because the buffer is unlikely to move, but since the whole
> exercise can only benefit crash-debugging scenarios, it'd be better
> to be more paranoid.

Good point.  This is supposed to be example code, so it shouldn't cut corners.

> One idea is to set debug_query_string immediately before each SPI_execute
> and clear it just afterwards, rather than trying to associate it with
> pgstat_report_activity calls.

Each elog(FATAL) check deserves a STATEMENT field if it fires, so I think that
would be too early to clear.  Here's an version fixing the defects.  In
worker_spi_main(), the timing now mirrors postgres.c.  initialize_worker_spi()
is doing something not directly possible from SQL, so I improvised there.