Horton

Re: On login trigger: take three

Daniel Gustafsson <daniel@yesql.se>

View thread View on postgr.es
From: Daniel Gustafsson <daniel@yesql.se>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Alexander Korotkov <aekorotkov@gmail.com>, Mikhail Gribkov <youzhick@gmail.com>, Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, Greg Nancarrow <gregn4422@gmail.com>, Ivan Panchenko <wao@mail.ru>, Teodor Sigaev <teodor@sigaev.ru>, Ibrar Ahmed <ibrar.ahmad@gmail.com>, vignesh C <vignesh21@gmail.com>, Pavel Stehule <pavel.stehule@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>
Date: 2023-10-03T13:43:38Z
Lists: pgsql-hackers 
> On 2 Oct 2023, at 20:10, Robert Haas <robertmhaas@gmail.com> wrote:
> 
> Sorry to have gone dark on this for a long time after having been
> asked for my input back in March. I'm not having a great time trying
> to keep up with email, and the threads getting split up makes it a lot
> worse for me.

Not a problem, thanks for chiming in.

> On Fri, Sep 29, 2023 at 6:15 AM Daniel Gustafsson <daniel@yesql.se> wrote:
>> Running the same pgbench command on my laptop looking at the average connection
>> times, and the averaging that over five runs (low/avg/high) I see ~5% increase
>> over master with the patched version (compiled without assertions and debug):
>> 
>> Patched event_triggers on:  6.858 ms/7.038 ms/7.434 ms
>> Patched event_triggers off: 6.601 ms/6.958 ms/7.539 ms
>> Master:                     6.676 ms/6.697 ms/6.760 ms
> 
> This seems kind of crazy to me. Why does it happen? It sounds to me
> like we must be doing a lot of extra catalog access to find out
> whether there are any on-login event triggers. Like maybe a sequential
> scan of pg_event_trigger.

That's exactly what happens, the patch is using BuildEventTriggerCache() to
build the hash for EVT which is then checked for login triggers.  This is
clearly the bottleneck and there needs to be a fast-path.  There used to be a
cache flag in an earlier version of the patch but it was a but klugy, a version
of that needs to be reimplemented for this patch to fly.

> I think a lot of users would say that logins on PostgreSQL are too slow already.

Agreed.

--
Daniel Gustafsson

Commits

  1. Fix some typos in event trigger docs

  2. Use heap_inplace_update() to unset pg_database.dathasloginevt

  3. Remove the flaky check in event_trigger_login regression test

  4. Fix instable 006_login_trigger.pl test

  5. Add support event triggers on authenticated login

  6. Add GUC for temporarily disabling event triggers

  7. Fix typo in reference to __FreeBSD__.

  8. Restore robustness of TAP tests that wait for postmaster restart.

  9. Restore the portal-level snapshot after procedure COMMIT/ROLLBACK.