Re: On login trigger: take three

čt 10. 12. 2020 v 19:09 odesílatel Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
napsal:

>
>
> čt 10. 12. 2020 v 16:48 odesílatel Konstantin Knizhnik <
> k(dot)knizhnik(at)postgrespro(dot)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,
>

but this information can be accessed via shared memory if it is necessary.
Probably it doesn't need a special hash table. Maybe a special flag per
process.

> 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(at)entry=1, v1=v1(at)entry=13836, v2=v2(at)entry=0,
> v3=v3(at)entry=0, v4=v4(at)entry=0) at catcache.c:1273
> #2 0x00000000008a7575 in SearchCatCache1 (cache=<optimized out>,
> v1=v1(at)entry=13836) at catcache.c:1167
> #3 0x00000000008b7f80 in SearchSysCache1 (cacheId=cacheId(at)entry=21,
> key1=key1(at)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(at)entry=1, argv=argv(at)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(at)entry=3,
> argv=argv(at)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
>
>
>
>