Hi, On 2019-08-01 22:42:23 +0200, Julien Rouhaud wrote: > On Thu, Aug 1, 2019 at 8:36 PM Andres Freund <and...@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 <and...@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
