čt 17. 12. 2020 v 14:04 odesílatel Konstantin Knizhnik < [email protected]> napsal:
> > > On 17.12.2020 9:31, Pavel Stehule wrote: > > > > st 16. 12. 2020 v 20:38 odesílatel Pavel Stehule <[email protected]> > napsal: > >> Attached please find new versoin of the patch based on >> on_connect_event_trigger_WITH_SUGGESTED_UPDATES.patch >> >>> So there is still only "disable_client_connection_trigger" GUC? because >>> we need possibility to disable client connect triggers and there is no such >>> need for other event types. >>> >>> As you suggested have added "dathaslogontriggers" flag which is set >>> when client connection trigger is created. >>> This flag is never cleaned (to avoid visibility issues mentioned in my >>> previous mail). >>> >> >> This is much better - I don't see any slowdown when logon trigger is not >> defined >> >> I did some benchmarks and looks so starting language handler is >> relatively expensive - it is about 25% and starting handler like event >> trigger has about 35%. But this problem can be solved later and elsewhere. >> >> I prefer the inverse form of disable_connection_trigger. Almost all GUC >> are in positive form - so enable_connection_triggger is better >> >> I don't think so current handling dathaslogontriggers is good for >> production usage. The protection against race condition can be solved by >> lock on pg_event_trigger >> > > I thought about it, and probably the counter of connect triggers will be > better there. The implementation will be simpler and more robust. > > > > > I prefer to implement different approach: unset dathaslogontriggers flag > in event trigger itself when no triggers are returned by > EventTriggerCommonSetup. > I am using double checking to prevent race condition. > The main reason for this approach is that dropping of triggers is not done > in event_trigger.c: it is done by generic code for all resources. > It seems to be there is no right place where decrementing of trigger > counters can be inserted. > Also I prefer to keep all logon-trigger specific code in one file. > > Also, as you suggested, I renamed disable_connection_trigger to > enable_connection_trigger. > > > New version of the patch is attached. > yes, it can work Regards Pavel > > > > -- > Konstantin Knizhnik > Postgres Professional: http://www.postgrespro.com > The Russian Postgres Company > >
