|From:||Andres Freund <andres(at)anarazel(dot)de>|
|To:||Julien Rouhaud <julien(dot)rouhaud(at)free(dot)fr>|
|Cc:||Evgeny Efimkin <efimkin(at)yandex-team(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>|
|Subject:||Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
On 2019-08-01 22:42:23 +0200, Julien Rouhaud wrote:
> On Thu, Aug 1, 2019 at 8:36 PM Andres Freund <andres(at)anarazel(dot)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(at)anarazel(dot)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.
|Next Message||Julien Rouhaud||2019-08-01 20:57:36||Re: The unused_oids script should have a reminder to use the 8000-8999 OID range|
|Previous Message||Julien Rouhaud||2019-08-01 20:49:48||Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?|