Horton

RE: [Proposal] Add foreign-server health checks infrastructure

Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>

View thread View on postgr.es
From: "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>
To: 'Fujii Masao' <masao.fujii@oss.nttdata.com>, 'Kyotaro Horiguchi' <horikyota.ntt@gmail.com>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, "Shinya11.Kato@oss.nttdata.com" <Shinya11.Kato@oss.nttdata.com>, "zyu@yugabyte.com" <zyu@yugabyte.com>
Date: 2022-02-24T02:34:55Z
Lists: pgsql-hackers

Attachments

Dear Fujii-san,

Thank you for your quick reviewing! I attached new version.
I found previous patches have wrong name. Sorry.

> The connection check timer is re-scheduled repeatedly even while the backend is
> in idle state or is running a local transaction that doesn't access to any foreign
> servers. I'm not sure if it's really worth checking the connections even in those
> states. Even without the periodic connection checks, if the connections are closed
> in those states, subsequent GetConnection() will detect that closed connection
> and re-establish the connection when starting remote transaction. Thought?

Indeed. We can now control the timer in fdw layer, so disable_timeout() was added
at the bottom of pgfdw_xact_callback(). 

> When a closed connection is detected in idle-in-transaction state and SIGINT is
> raised, nothing happens because there is no query running to be canceled by
> SIGINT. Also in this case the connection check timer gets disabled. So we can still
> execute queries that don't access to foreign servers, in the same transaction, and
> then the transaction commit fails. Is this expected behavior?

It's not happy, but I'm not sure about a good solution. I made a timer reschedule
if connection lost had detected. But if queries in the transaction are quite short,
catching SIGINT may be fail.

> When I shutdowned the foreign server while the local backend is in
> idle-in-transaction state, the connection check timer was triggered and detected
> the closed connection. Then when I executed COMMIT command, I got the
> following WARNING message. Is this a bug?
> 
>      WARNING:  leaked hash_seq_search scan for hash table 0x7fd2ca878f20

Fixed. It is caused because hash_seq_term() was not called when checker detects
a connection lost.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Commits

  1. postgres_fdw: Extend postgres_fdw_get_connections to return user name.

  2. postgres_fdw: Fix bug in connection status check.

  3. postgres_fdw: Add connection status check to postgres_fdw_get_connections().

  4. postgres_fdw: Add "used_in_xact" column to postgres_fdw_get_connections().

  5. doc: Enhance documentation for postgres_fdw_get_connections() output columns.

  6. Avoid reference to nonexistent array element in ExecInitAgg().

  7. Add WL_SOCKET_CLOSED for socket shutdown events.

  8. postgres_fdw: reestablish new connection if cached one is detected as broken.