Re: On login trigger: take three

Hi hackers,

Since the original authors, as Daniel said, seems to have retired from the
patch, I have allowed myself to continue the patch polishing.

Attached v32 includes fresh rebase and the following fixes:

- Copying dathasloginevt flag during DB creation from template;

- Restoring dathasloginevt flag after re-enabling a disabled trigger;

- Preserving dathasloginevt flag during not-running a trigger because of
some filters (running with 'session_replication_role=replica' for example);

- Checking tags during the trigger creation;

The (en/dis)abling GUC was removed from the patch by default because it is
now discussed and supported in a separate thread by Daniel Gustaffson:

*https://www.postgresql.org/message-id/9140106E-F9BF-4D85-8FC8-F2D3C094A6D9@yesql.se*
<https://www.postgresql.org/message-id/9140106E-F9BF-4D85-8FC8-F2D3C094A6D9@yesql.se>

Still the back-ported version of the Daniel’s patch is attached here too –
just for convenience.

While the above changes represent the straight work continue, there is
another version to consider. Most of the patch problems seem to originate
from the dathasloginevt flag support. There are lots of known problems
(partly fixed) and probably a lot more unfound. At the same time the flag
itself is not a critical element, but just a performance optimizer for
logging-in of users who are NOT using the on-login triggers.

I have made a simpler version without the flag and tried to compare the
performance. The results show that life without dathasloginevt is still
possible. When there is a small number of non-login event triggers, the
difference is barely noticeable indeed. To show the difference, I have
added 1000 sql_drop event triggers and used pgbench to continuously query
the database for “select 1” with reconnects for 3 seconds. Here are the
results. For each version I recorded average connection time with 0 and
with 1000 event triggers:

With dathasloginevt flag: 0.609 ms VS 0.611 ms

No-flag version: 0.617 ms VS 0.727 ms

You can see that the difference is noticeable, but not that critical as we
can expect for 1000 triggers (while in a vast majority of real cases there
would be a much lesser number of triggers). I.e. the flagless version of
the patch introduces a huge reliability raise at the cost of a small
performance drop.

What do you think? Maybe it’s better to use the flagless version? Or even a
small performance drop is considered as unacceptable?

The attached files include the two versions of the patch: “v32” for the
updated version with flag and “v31_nf” – for the flagless version. Both
versions contain “0001“ file for the patch itself and “0002” for
back-ported controlling GUC.
--
best regards,
Mikhail A. Gribkov

On Fri, Nov 4, 2022 at 3:58 AM Ian Lawrence Barwick <barwick(at)gmail(dot)com>
wrote:

> 2022年11月4日(金) 5:23 Daniel Gustafsson <daniel(at)yesql(dot)se>:
> >
> > > On 20 Sep 2022, at 16:10, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
> >
> > > Since the original authors seems to have retired from the patch
> > > (I've only rebased it to try and help) I am inclined to mark it as
> returned
> > > with feedback.
> >
> > Doing so now since no updates have been posted for quite some time and
> holes
> > have been poked in the patch.
>
> I see it was still "Waiting on Author" so I went ahead and changed the
> status.
>
> > The GUC for temporarily disabling event triggers, to avoid the need for
> single-
> > user mode during troubleshooting, might however be of interest so I will
> break
> > that out and post to a new thread.
>
> For reference this is the new thread:
>
>
> https://www.postgresql.org/message-id/9140106E-F9BF-4D85-8FC8-F2D3C094A6D9%40yesql.se
>
> Regards
>
> Ian Barwick
>
>
>