On Wed, Feb 1, 2023 at 12:46 PM Sergey Dudoladov <sergey.dudola...@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
