Re: On login trigger: take three
Pavel Stehule <pavel.stehule@gmail.com>
From: Pavel Stehule <pavel.stehule@gmail.com>
To: Konstantin Knizhnik <k.knizhnik@postgrespro.ru>
Cc: Greg Nancarrow <gregn4422@gmail.com>,
PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2020-12-10T18:09:07Z
Lists: pgsql-hackers
čt 10. 12. 2020 v 16:48 odesílatel Konstantin Knizhnik <
k.knizhnik@postgrespro.ru> napsal:
>
>
> On 10.12.2020 18:12, Pavel Stehule wrote:
>
>
> My idea was a little bit different. Inside postinit initialize some global
> variables with info if there are event triggers or not. And later you can
> use this variable to start transactions and other things.
>
> There will be two access to pg_event_trigger, but it can eliminate useless
> and probably more expensive start_transaction and end_transaction.
>
>
>
> Do you mean some variable in shared memory or GUCs?
> It was my first idea - to use some flag in shared memory to make it
> possible fast check that there are not event triggers.
> But this flag should be sent per database. May be I missed something, but
> there is no any per-database shared memory data structure in Postgres.
> Certainly it is possible to organize some hash db->event_info, but it
> makes this patch several times more complex.
>
My idea was a little bit different - just inside process initialization,
checking existence of event triggers, and later when it is necessary to
start a transaction for trigger execution. This should to reduce useless
empty transaction,
I am sure this is a problem on computers with slower CPU s, although I have
I7, but because this feature is usually unused, then the performance impact
should be minimal every time.
>
> From my point of view it is better to have separate GUC disabling just
> client connection events and switch it on by default.
> So only those who need this events with switch it on, other users will not
> pay any extra cost for it.
>
It can work, but this design is not user friendly. The significant
bottleneck should be forking new processes, and check of content some
usually very small tables should be invisible. So if there is a possible
way to implement some feature that can be enabled by default, then we
should go this way. It can be pretty unfriendly if somebody writes a logon
trigger and it will not work by default without any warning.
When I tested last patch I found a problem (I have disabled assertions due
performance testing)
create role xx login;
alter system set disable_event_triggers to on; -- better should be positive
form - enable_event_triggers to on, off
select pg_reload_conf();
psql -U xx
Thread 2.1 "postmaster" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7f2bff438ec0 (LWP 827497)]
ResourceOwnerEnlargeCatCacheRefs (owner=0x0) at resowner.c:1025
1025 ResourceArrayEnlarge(&(owner->catrefarr));
(gdb) bt
#0 ResourceOwnerEnlargeCatCacheRefs (owner=0x0) at resowner.c:1025
#1 0x00000000008a70f8 in SearchCatCacheInternal (cache=<optimized out>,
nkeys=nkeys@entry=1, v1=v1@entry=13836, v2=v2@entry=0,
v3=v3@entry=0, v4=v4@entry=0) at catcache.c:1273
#2 0x00000000008a7575 in SearchCatCache1 (cache=<optimized out>,
v1=v1@entry=13836) at catcache.c:1167
#3 0x00000000008b7f80 in SearchSysCache1 (cacheId=cacheId@entry=21,
key1=key1@entry=13836) at syscache.c:1122
#4 0x00000000005939cd in pg_database_ownercheck (db_oid=13836,
roleid=16387) at aclchk.c:5114
#5 0x0000000000605b42 in EventTriggersDisabled () at event_trigger.c:650
#6 EventTriggerOnConnect () at event_trigger.c:839
#7 0x00000000007b46d7 in PostgresMain (argc=argc@entry=1,
argv=argv@entry=0x7ffdd6256080,
dbname=<optimized out>,
username=0xf82508 "tom") at postgres.c:4021
#8 0x0000000000741afd in BackendRun (port=<optimized out>, port=<optimized
out>) at postmaster.c:4490
#9 BackendStartup (port=<optimized out>) at postmaster.c:4212
#10 ServerLoop () at postmaster.c:1729
#11 0x0000000000742881 in PostmasterMain (argc=argc@entry=3,
argv=argv@entry=0xf44d00)
at postmaster.c:1401
#12 0x00000000004f1ff7 in main (argc=3, argv=0xf44d00) at main.c:209
I checked profiles, and although there is significant slowdown when there
is one login trigger, it was not related to plpgsql.
There is big overhead of
8,98% postmaster postgres [.] guc_name_compare
Maybe EventTriggerInvoke is not well optimized for very frequent execution.
Overhead of plpgsql is less than 0.1%, although there is significant
slowdown (when a very simple logon trigger is executed).
Regards
Pavel
Commits
-
Fix some typos in event trigger docs
- 5fce30e77fe1 17.0 landed
-
Use heap_inplace_update() to unset pg_database.dathasloginevt
- 8be93177c46b 17.0 landed
-
Remove the flaky check in event_trigger_login regression test
- 4b885d01f967 17.0 landed
-
Fix instable 006_login_trigger.pl test
- 06be01eb266b 17.0 landed
-
Add support event triggers on authenticated login
- e83d1b0c40cc 17.0 landed
-
Add GUC for temporarily disabling event triggers
- 7750fefdb2b8 17.0 landed
-
Fix typo in reference to __FreeBSD__.
- e52f8b301ed5 16.0 cited
-
Restore robustness of TAP tests that wait for postmaster restart.
- f452aaf7d4a9 14.0 cited
-
Restore the portal-level snapshot after procedure COMMIT/ROLLBACK.
- 84f5c2908dad 14.0 cited