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