Re: On login trigger: take three

From: Konstantin Knizhnik <k(dot)knizhnik(at)postgrespro(dot)ru>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Greg Nancarrow <gregn4422(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: On login trigger: take three
Date: 2020-12-17 13:03:56
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 17.12.2020 9:31, Pavel Stehule wrote:
> st 16. 12. 2020 v 20:38 odesílatel Pavel Stehule
> <pavel(dot)stehule(at)gmail(dot)com <mailto:pavel(dot)stehule(at)gmail(dot)com>> 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
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

New version of the patch is attached.

Konstantin Knizhnik
Postgres Professional:
The Russian Postgres Company

Attachment Content-Type Size
on_connect_event_trigger-9.patch text/x-patch 29.2 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Konstantin Knizhnik 2020-12-17 13:05:18 Re: On login trigger: take three
Previous Message Amit Kapila 2020-12-17 12:54:42 Re: [HACKERS] logical decoding of two-phase transactions