Re: System username in pg_stat_activity

On Mon, Jan 15, 2024 at 11:17 AM Bertrand Drouvot
<bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Fri, Jan 12, 2024 at 05:16:53PM +0100, Magnus Hagander wrote:
> > On Thu, Jan 11, 2024 at 5:55 PM Bertrand Drouvot
> > <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
> > >
> > > I'm wondering if it would make sense to populate it for parallel workers too.
> > > I think it's doable thanks to d951052, but I'm not sure it's worth it (one could
> > > join based on the leader_pid though). OTOH that would be consistent with
> > > how the SYSTEM_USER behaves with parallel workers (it's populated).
> >
> > I guess one could conceptually argue that "authentication happens int
> > he leader". But we do populate it with the other user records, and
> > it'd be weird if this one was excluded.
> >
> > The tricky thing is that pgstat_bestart() is called long before we
> > deserialize the data. But from what I can tell it should be safe to
> > change it per the attached? That should be AFAICT an extremely short
> > window of time longer before we report it, not enough to matter.
>
> Thanks! Yeah, that seems reasonable to me. Also, I think we should remove the
> "MyProcPort" test here then (looking at v3):
>
> + if (MyProcPort && MyClientConnectionInfo.authn_id)
> + strlcpy(lbeentry.st_auth_identity, MyClientConnectionInfo.authn_id, NAMEDATALEN);
> + else
> + MemSet(&lbeentry.st_auth_identity, 0, sizeof(lbeentry.st_auth_identity));
>
> to get the st_auth_identity propagated to the parallel workers.

Yup, I had done that in v4 which as you noted further down, I forgot to post.

> > > - what about "Contains the same value as the identity part in <xref linkend="system-user" />"?
>
> Not sure, but looks like you missed this comment?

I did. Agree with your comment, and updated now.

> > > +# Users with md5 auth should show both auth method and name in pg_stat_activity
> > >
> > > what about "show both auth method and identity"?
> >
> > Good spot, yeah, I changed it over to identity everywhere else so it
> > should be here as well.
>
> Did you forget to share the new revision (aka v4)? I can only see the
> "reorder_parallel_worker_bestart.patch" attached.

I did. Here it is, and also including that suggested docs fix as well
as a rebase on current master.

--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/