Re: On login trigger: take three

> On 25 Sep 2023, at 11:13, Alexander Korotkov <aekorotkov@gmail.com> wrote:

> I'd like to do a short summary of
> design issues on this thread.

Thanks for summarizing this long thread!

> 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].

I have committed the prerequisite patch for temporarily disabling EVTs via a
GUC in 7750fefdb2.  We should absolutely avoid creating any more dependencies
on single-user mode (yes, I have changed my mind since the beginning of the
thread).

> 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.

While I disliked the flag, I do think the overhead is problematic.  Last time I
profiled it I found it noticeable, and it seems expensive for such a niche
feature to impact such a hot path.  Maybe you can think of other ways to reduce
the cost here (if it indeed is an issue in the latest version of the patch,
which is not one I've benchmarked)?

> 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.

While it is a psql problem, it's exacerbated by a slow login EVT and it breaks
what I would guess is the mental model of many who press ^C in a stalled login.
At the very least we should probably document the risk?

--
Daniel Gustafsson