Re: Add connection active, idle time to pg_stat_activity - Mailing list pgsql-hackers

From Andrey Borodin
Subject Re: Add connection active, idle time to pg_stat_activity
Date
Msg-id CAAhFRxibrZkLmJnH30T8dTK4JtQXCbmnquU9QvBg0vS18iu8wg@mail.gmail.com
Whole thread Raw
In response to Re: Add connection active, idle time to pg_stat_activity  (Sergey Dudoladov <sergey.dudoladov@gmail.com>)
Responses Re: Add connection active, idle time to pg_stat_activity
List pgsql-hackers
On Wed, Feb 1, 2023 at 12:46 PM Sergey Dudoladov
<sergey.dudoladov@gmail.com> wrote:
>
> I've sketched the first version of a patch to add pg_stat_session.
> Please review this early version.

Hi Sergey!

I've taken a look into the patch and got some notes.
1. It is hard to understand what fastpath backend state is. What do
fastpath metrics mean for a user?
2. Anyway, the path "if (PGSTAT_IS_FASTPATH(beentry))" seems
unreachable to me. I'm a bit surprised that compilers do not produce
warnings about it. Maybe I'm just wrong.
3. Tests do not check any incrementation logic. I think we can have
some test that verifies delta for select some_counter from
pg_stat_session where pid = pg_backend_pid();
4. Macroses like PGSTAT_IS_RUNNING do not look like net win in code
readability and PGSTAT prefix have no semantic load.


That's all I've found so far. Thank you!

Best regards, Andrey Borodin.

PS. We were doing on-air review session [0], I hope Nik will chime-in
with "usability part of a review".

[0] https://youtu.be/vTV8XhWf3mo?t=2404



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: Missing free_var() at end of accum_sum_final()?
Next
From: "Jonah H. Harris"
Date:
Subject: Reducing System Allocator Thrashing of ExecutorState to Alleviate FDW-related Performance Degradations