Re: Patch proposal: New hooks in the connection path

On Tue, Aug 2, 2022 at 6:55 PM Drouvot, Bertrand <bdrouvot(at)amazon(dot)com> wrote:
>
> Hi Bharath,
>
> On 7/14/22 11:43 AM, Bharath Rupireddy wrote:
> > On Fri, Jul 8, 2022 at 5:54 PM Bharath Rupireddy
> > <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> >> Looking at v2-0003 patch and emit_log_hook, how about we filter out
> >> for those connectivity errors either based on error codes and if they
> >> aren't unique, perhaps passing special flags to ereport API indicating
> >> that it's a connectivity error and in the emit_log_hook we can look
> >> for those connectivity error codes or flags to collect the stats about
> >> the failure connections (with MyProcPort being present in
> >> emit_log_hook)? This way, we don't need a new hook. Thoughts?
> > Bertrand and Other Hackers, above comment may have been lost in the
> > wild - any thoughts on it?
>
> Thanks for your feedback!
>
> I can see 2 issues with that approach:
>
> - We’ll not be able to track the “startup timeout case” (well, we may
> not be able to track it anyway depending of what next to [1] will be) as
> it does not emit any log messages.
>
> [1]:
> https://www.postgresql.org/message-id/a1558d12-c1c4-0fe5-f8a5-2b6c2294e55f%40amazon.com

Yes, we wanted to be very quick in StartupPacketTimeoutHandler because
it is a timeout signal handler after all.

> - We’ll depend of the log_min_messages value (means
> edata->output_to_server needs to be true for the emit_log_hook to be
> triggered).

Hm, we can just say that 'log_min_message setting will enable/disable
the feature'.

I agree with your first point of not having an error in
StartupPacketTimeoutHandler hence I don't think using emit log hook
for the connection failure stats helps.

--
Bharath Rupireddy
RDS Open Source Databases: https://aws.amazon.com/rds/postgresql/