Hi, 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? I mean, we do so at a frequency roughtly as high as high as the new queryid updates for things like pgstat_report_activity(). Reading the value of your own backend you'd not need to follow the changecount algorithm, I think, because it's only updated from the current backend. If reading were a problem, you trivially just could have a cache in a local variable, to avoid accessing shared memory. > Is widening PGPROC is too expensive for this purpose? Well, I'm mostly not a fan of putting even more in there, because it's pretty hard to understand already. To me it architecturally status information doesn't belong there (In fact, I'm somewhat unhappy that wait_event_info etc in there, but that's at least commonly updated at the same time as other fields in PGPROC). Greetings, Andres Freund
