Re: On login trigger: take three

Hi!

On Wed, Jun 14, 2023 at 10:49 PM Mikhail Gribkov <youzhick(at)gmail(dot)com> wrote:
> The attached v40 patch is a fresh rebase on master branch to actualize the state before the upcoming commitfest.
> Nothing has changed beyond rebasing.
>
> And just for convenience, here is a link to the exact message of the thread
> describing the current approach:
> https://www.postgresql.org/message-id/CAMEv5_vg4aJOoUC74XJm%2B5B7%2BTF1nT-Yhtg%2BawtBOESXG5Grfg%40mail.gmail.com

Thank you for the update. I think the patch is interesting and
demanding. The code, docs and tests seem to be quite polished
already. Simultaneously, the thread is long and spread over a long
time. So, it's easy to lose track. I'd like to do a short summary of
design issues on this thread.

1. Initially the patch introduced "on login" triggers. It was unclear
if they need to rerun on RESET ALL or DISCARD ALL [1]. The new name is
"client connection" trigger, which seems fine [2].

2. Another question is how to deal with triggers, which hangs or fails
with error [1]. One possible way to workaround that is single-user
mode, which is already advised to workaround the errors in other event
triggers. However, in perspective single-user mode might be deleted.
Also, single-user mode is considered as a worst-case scenario recovery
tool, while it's very easy to block the database connections with
client connection triggers. As addition/alternative to single-user
mode, GUC options to disable all event triggers and/or client
connection triggers. Finally, the patch for the GUC option to disable
all event triggers resides in a separate thread [4]. Apparently that
patch should be committed first [5].

3. Yet another question is connection-time overhead introduced by this
patch. The overhead estimate varies from no measurable overhead [6] to
5% overhead [7]. In order to overcome that, [8] has introduced a
database-level flag indicating whether there are connection triggers.
Later this flag was removed [9] in a hope that the possible overhead
is acceptable.

4. [10] points that there is no clean way to store information about
unsuccessful connections (declined by either authentication or
trigger). However, this is considered out-of-scope for the current
patch, and could be implemented later if needed.

5. It was also pointed out [11] that ^C in psql doesn't cancel
long-running client connection triggers. That might be considered a
psql problem though.

6. It has been also pointed out that [12] all triggers, which write
data to the database, must check pg_is_in_recovery() to work correctly
on standby. That seems to be currently reflected in the documentation.

So, for me the open issues seem to be 2, 3 and 5. My plan to revive
this patch is to commit the GUC patch [4], recheck the overhead and
probably leave "^C in psql" problem as a separate standalone issue.
Any thoughts?

Links.

1. https://www.postgresql.org/message-id/CAFj8pRBdqdqvkU3mVKzoOnO+jPz-6manRV47CDEa+1jD6x6LFg%40mail.gmail.com
2. https://www.postgresql.org/message-id/CAFj8pRCxdQgHy8Mynk3hz6pFsqQ9BN6Vfgy0MJLtQBAUhWDf3w%40mail.gmail.com
3. https://www.postgresql.org/message-id/E0D5DC61-C490-45BD-A984-E8D56493EC4F%40yesql.se
4. https://www.postgresql.org/message-id/9140106E-F9BF-4D85-8FC8-F2D3C094A6D9@yesql.se
5. https://www.postgresql.org/message-id/20230306221010.gszjoakt5jp7oqpd%40awork3.anarazel.de
6. https://www.postgresql.org/message-id/90760f2d-2f9c-12ab-d2c5-e8e6fb7d08de%40postgrespro.ru
7. https://www.postgresql.org/message-id/CAFj8pRChwu01VLx76nKBVyScHCsd1YnBGiKfDJ6h17g4CSnUBg%40mail.gmail.com
8. https://www.postgresql.org/message-id/4471d472-5dfc-f2b0-ad05-0ff8d0a3bb0c%40postgrespro.ru
9. https://www.postgresql.org/message-id/CAMEv5_vg4aJOoUC74XJm%2B5B7%2BTF1nT-Yhtg%2BawtBOESXG5Grfg%40mail.gmail.com
10. https://www.postgresql.org/message-id/9c897136-4755-dcfc-2d24-b12bcfe4467f%40sigaev.ru
11. https://www.postgresql.org/message-id/CA%2BTgmoZv9f1s797tihx-zXQN4AE4ZFBV5C0K%3DzngbgNu3xNNkg%40mail.gmail.com
12. https://www.postgresql.org/message-id/20220312024652.lvgehszwke4hhove%40alap3.anarazel.de

------
Regards,
Alexander Korotkov