Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?

From: Nitin Jadhav <nitinjadhavpostgres(at)gmail(dot)com>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Julien Rouhaud <rjuju123(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Hannu Krosing <hannuk(at)google(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Paquier <michael(at)paquier(dot)xyz>, torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>, Atsushi Torikoshi <atorik(at)gmail(dot)com>, Tatsuro Yamada <tatsuro(dot)yamada(dot)tf(at)nttcom(dot)co(dot)jp>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, 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?
Date: 2021-04-06 14:35:19
Message-ID: CAMm1aWZBBPPjRau9dP6+PZFoirKMvy3tX5uqrY-wF0PgR9KvSg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I have reviewed the code. Here are a few minor comments.

1.
+void
+pgstat_report_queryid(uint64 queryId, bool force)
+{
+ volatile PgBackendStatus *beentry = MyBEEntry;
+
+ if (!beentry)
+ return;
+
+ /*
+ * if track_activities is disabled, st_queryid should already have been
+ * reset
+ */
+ if (!pgstat_track_activities)
+ return;

The above two conditions can be clubbed together in a single condition.

2.
+/* ----------
+ * pgstat_get_my_queryid() -
+ *
+ * Return current backend's query identifier.
+ */
+uint64
+pgstat_get_my_queryid(void)
+{
+ if (!MyBEEntry)
+ return 0;
+
+ return MyBEEntry->st_queryid;
+}

Is it safe to directly read the data from MyBEEntry without
calling pgstat_begin_read_activity() and pgstat_end_read_activity(). Kindly
ref pgstat_get_backend_current_activity() for more information. Kindly let
me know if I am wrong.

Thanks and Regards,
Nitin Jadhav

On Mon, Apr 5, 2021 at 10:46 PM Bruce Momjian <bruce(at)momjian(dot)us> wrote:

> On Sun, Apr 4, 2021 at 10:18:50PM +0800, Julien Rouhaud wrote:
> > On Fri, Apr 02, 2021 at 01:33:28PM +0800, Julien Rouhaud wrote:
> > > On Thu, Apr 01, 2021 at 03:27:11PM -0400, Bruce Momjian wrote:
> > > >
> > > > OK, I am happy with your design decisions, thanks.
> > >
> > > Thanks! While double checking I noticed that I failed to remove a
> (now)
> > > useless include of pgstat.h in nodeGatherMerge.c in last version. I'm
> > > attaching v22 to fix that, no other change.
> >
> > There was a conflict since e1025044c (Split backend status and progress
> related
> > functionality out of pgstat.c).
> >
> > Attached v23 is a rebase against current HEAD, and I also added a few
> > UINT64CONST() macro usage for consistency.
>
> Thanks. I struggled with merging the statistics collection changes into
> my cluster file encryption branches because my patch made changes to
> code that moved to another C file.
>
> I plan to apply this tomorrow.
>
> --
> Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
> EDB https://enterprisedb.com
>
> If only the physical world exists, free will is an illusion.
>
>
>
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Himanshu Upadhyaya 2021-04-06 14:47:57 [PATCH] PREPARE TRANSACTION unexpected behavior with TEMP TABLE
Previous Message Bharath Rupireddy 2021-04-06 14:25:27 Re: TRUNCATE on foreign table