Hi,
On 2019-08-01 22:42:23 +0200, Julien Rouhaud wrote:
> On Thu, Aug 1, 2019 at 8:36 PM Andres Freund <andres@anarazel.de> wrote:
> >
> > On 2019-08-01 08:45:45 +0200, Julien Rouhaud wrote:
> > > On Wed, Jul 31, 2019 at 11:59 PM Andres Freund <andres@anarazel.de> wrote:
> > > > And if it were necessary, why wouldn't any of the other fields in
> > > > PgBackendStatus need it? There's plenty of other fields written to
> > > > without a lock, and several of those are also 8 bytes (so it's not a
> > > > case of assuming that 8 byte reads might not be atomic, but for byte
> > > > reads are).
> > >
> > > This patch is actually storing the queryid in PGPROC, not in
> > > PgBackendStatus, thus the need for an atomic. I used PGPROC because
> > > the value needs to be available in log_line_prefix() and spi.c, so
> > > pgstat.c / PgBackendStatus didn't seem like the best interface in that
> > > case.
> >
> > Hm. I'm not convinced that really is the case? You can just access
> > MyBEentry, and read and update it?
>
> Sure, but it requires extra wrapper functions, and the st_changecount
> dance when writing the new value.
So? You need a wrapper function anyway, there's no way we're going to
add all those separate pg_atomic_write* calls directly.
> > I mean, we do so at a frequency
> > roughtly as high as high as the new queryid updates for things like
> > pgstat_report_activity().
>
> pgstat_report_activity() is only called for top-level statement. For
> the queryid we need to track it down to all nested statements, which
> could be way higher.
Compared to the overhead of executing a separate query the cost of
single function call containing a MyBEentry update of an 8byte value
seems almost guaranteed to be immeasurable. The executor startup alone
is several orders of magnitude more expensive.
I also think this proposed column should probably respect
the track_activities GUC.
Greetings,
Andres Freund