Hi po 21. 12. 2020 v 11:06 odesílatel Konstantin Knizhnik < [email protected]> napsal:
> > > On 20.12.2020 10:04, Pavel Stehule wrote: > > > > čt 17. 12. 2020 v 19:30 odesílatel Pavel Stehule <[email protected]> > napsal: > >> >> >> č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 >> > > for complete review the new field in pg_doc should be documented > > > Done. > Also I fixed some examples in documentation which involves pg_database. > regress tests fails sysviews ... FAILED 112 ms test event_trigger ... FAILED (test process exited with exit code 2) 447 ms test fast_default ... FAILED 392 ms test stats ... FAILED 626 ms ============== shutting down postmaster ============== Regards Pavel > -- > Konstantin Knizhnik > Postgres Professional: http://www.postgrespro.com > The Russian Postgres Company > >
