Hi,
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? 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