Re: On login trigger: take three - Mailing list pgsql-hackers
| From | Mikhail Gribkov |
|---|---|
| Subject | Re: On login trigger: take three |
| Date | |
| Msg-id | CAMEv5_vg4aJOoUC74XJm+5B7+TF1nT-Yhtg+awtBOESXG5Grfg@mail.gmail.com Whole thread Raw |
| In response to | Re: On login trigger: take three (Mikhail Gribkov <youzhick@gmail.com>) |
| Responses |
Re: On login trigger: take three
(Nikita Malakhov <hukutoc@gmail.com>)
|
| List | pgsql-hackers |
Hi hackers,
Attached v33 is a new rebase of the flagless version of the patch. As there were no objections at first glance, I’ll try to post it to the upcoming commitfest, thus the brief recap of all the patch details is below.
v33-On_client_login_event_trigger
The patch introduces a trigger on login event, allowing to fire some actions right on the user connection. This can be useful for logging or connection check purposes as well as for some personalization of the environment. Usage details are described in the documentation included, but shortly usage is the same as for other triggers: create function returning event_trigger and then create event trigger on login event.
The patch is prepared to be applied to the master branch and tested when applied after e52f8b301ed54aac5162b185b43f5f1e44b6b17e commit by Thomas Munro (Date: Fri Dec 16 17:36:22 2022 +1300).
Regression tests and documentation included.
A couple of words about what and why I changed compared to the previous author's version.
First, the (en/dis)abling GUC was removed from the patch because ideologically it is a separate feature and nowadays it’s discussed and supported in a separate thread by Daniel Gustaffson:
https://www.postgresql.org/message-id/flat/9140106E-F9BF-4D85-8FC8-F2D3C094A6D9%40yesql.se
Second, I have removed the dathasloginevt flag. The flag was initially added to the patch for performance reasons and it did the job, although it was quite a clumsy construct causing tons of bugs and could potentially lead to tons more during further PostgreSQL evolution. I have removed the flag and found that the performance drop is not that significant.
And this is an aspect I should describe in more detail.
The possible performance drop is expected as an increased time used to login a user NOT using a login trigger.
First of all, the method of performance check:
echo 'select 1;' > ./tst.sql
pgbench -n -C -T3 -f tst.sql -U postgres postgres
The output value "average connection time" is the one I use to compare performance.
Now, what are the results.
* master branch: 0.641 ms
* patched version: 0.644 ms
No significant difference here and it is just what was expected. Based on the patch design the performance drop can be expected when there are lots of event triggers created, but not the login one. Thus I have created 1000 drop triggers (the script for creating triggers is attached too) in the database and repeated the test:
* master branch: 0.646 ms
* patched version: 0.754 ms
For 2000 triggers the patched version connection time is further increased to 0.862. Thus we have a login time rise in about 16.5% per 1000 event triggers in the database. It is a statistically noticeable value, still I don’t think it’s a critical one we should be afraid of.
N.B. The exact values of the login times slightly differ from the ones I posted in the previous email. Well, that’s the repeatability level we have. This convinces me even more that the observed level of performance drop is acceptable.
Attached v33 is a new rebase of the flagless version of the patch. As there were no objections at first glance, I’ll try to post it to the upcoming commitfest, thus the brief recap of all the patch details is below.
v33-On_client_login_event_trigger
The patch introduces a trigger on login event, allowing to fire some actions right on the user connection. This can be useful for logging or connection check purposes as well as for some personalization of the environment. Usage details are described in the documentation included, but shortly usage is the same as for other triggers: create function returning event_trigger and then create event trigger on login event.
The patch is prepared to be applied to the master branch and tested when applied after e52f8b301ed54aac5162b185b43f5f1e44b6b17e commit by Thomas Munro (Date: Fri Dec 16 17:36:22 2022 +1300).
Regression tests and documentation included.
A couple of words about what and why I changed compared to the previous author's version.
First, the (en/dis)abling GUC was removed from the patch because ideologically it is a separate feature and nowadays it’s discussed and supported in a separate thread by Daniel Gustaffson:
https://www.postgresql.org/message-id/flat/9140106E-F9BF-4D85-8FC8-F2D3C094A6D9%40yesql.se
Second, I have removed the dathasloginevt flag. The flag was initially added to the patch for performance reasons and it did the job, although it was quite a clumsy construct causing tons of bugs and could potentially lead to tons more during further PostgreSQL evolution. I have removed the flag and found that the performance drop is not that significant.
And this is an aspect I should describe in more detail.
The possible performance drop is expected as an increased time used to login a user NOT using a login trigger.
First of all, the method of performance check:
echo 'select 1;' > ./tst.sql
pgbench -n -C -T3 -f tst.sql -U postgres postgres
The output value "average connection time" is the one I use to compare performance.
Now, what are the results.
* master branch: 0.641 ms
* patched version: 0.644 ms
No significant difference here and it is just what was expected. Based on the patch design the performance drop can be expected when there are lots of event triggers created, but not the login one. Thus I have created 1000 drop triggers (the script for creating triggers is attached too) in the database and repeated the test:
* master branch: 0.646 ms
* patched version: 0.754 ms
For 2000 triggers the patched version connection time is further increased to 0.862. Thus we have a login time rise in about 16.5% per 1000 event triggers in the database. It is a statistically noticeable value, still I don’t think it’s a critical one we should be afraid of.
N.B. The exact values of the login times slightly differ from the ones I posted in the previous email. Well, that’s the repeatability level we have. This convinces me even more that the observed level of performance drop is acceptable.
--
Attachment
pgsql-hackers by date: