Horton

Re: Patch proposal: New hooks in the connection path

bdrouvotAWS <bdrouvot@amazon.com>

View thread
From: "Drouvot, Bertrand" <bdrouvot@amazon.com>
To: Tom Lane <tgl@sss.pgh.pa.us>, Joe Conway <mail@joeconway.com>
Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, Nathan Bossart <nathandbossart@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-07-11T06:18:46Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add pg_stat_database counters for sessions and session time

Hi,

On 7/7/22 10:10 PM, Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>> It isn't clear to me if having a hook in the timeout handler is a
>> nonstarter -- perhaps a comment with suitable warning for prospective
>> extension authors is enough? Anyone else want to weigh in on this issue
>> specifically?
> It doesn't seem like a great place for a hook, because the list of stuff
> you could safely do there would be mighty short, possibly the empty set.
> Write to shared memory?  Not too safe.  Write to a file?  Even less.
> Write to local memory?  Pointless, because we're about to _exit(1).
> Pretty much anything I can think of that you'd want to do is something
> we've already decided the core code can't safely do, and putting it
> in a hook won't make it safer.
>
> If someone wants to argue for this hook, I'd like to see a credible
> example of a *safe* use-case, keeping in mind the points raised in
> the comments in BackendInitialize and process_startup_packet_die.

The use case would be to increment a counter in shared memory (or most 
probably within an hash table) to reflect the number of times a startup 
packet timeout occurred.

Reading the comments in/related to BackendInitialize() I understand 
that's definitely not safe to write in shared memory for the 
EXEC_BACKEND case, but wouldn't it be safe for the non EXEC_BACKEND case?

BTW, it makes me realize that the hook being fired in the bad startup 
packet case:

         /*
          * Stop here if it was bad or a cancel packet. ProcessStartupPacket
          * already did any appropriate error reporting.
          */
         if (status != STATUS_OK)
+       {
+               if (FailedConnection_hook)
+                       (*FailedConnection_hook) 
(FCET_BAD_STARTUP_PACKET, port);
                 proc_exit(0);
+       }

is not safe for the EXEC_BACKEND case.

Regards,

-- 
Bertrand Drouvot
Amazon Web Services: https://aws.amazon.com