On Fri, Sep 25, 2020 at 3:21 PM Fujii Masao <masao.fu...@oss.nttdata.com> wrote: > > I think that we can simplify the code by merging the connection-retry > code into them, like the attached very WIP patch (based on yours) does. >
+1. > > + else > + ereport(ERROR, > + (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION), > + errmsg("could not connect to server \"%s\"", > + server->servername), > + errdetail_internal("%s", pchomp(PQerrorMessage(entry->conn))))); > > The above is not necessary? If this error occurs, connect_pg_server() > reports an error, before the above code is reached. Right? > Removed. Thanks for the comments. I think we need to have a volatile qualifier for need_reset_conn, because of the sigsetjmp. Instead of "need_reset_conn", "retry_conn" would be more meaningful and also instead of goto label name "reset;", "retry:". I changed "closing connection %p to reestablish connection" to "closing connection %p to reestablish a new one" I also adjusted the comments to be under the 80char limit. I feel, when we are about to close an existing connection and reestablish a new connection, it will be good to have a debug3 message saying that we "could not connect to postgres_fdw connection %p"[1]. Attaching v5 patch that has the above changes. Both make check and make check-world regression tests passes. Please review. [1] This would tell the user that we are not able to connect to the connection. postgres=# SELECT * FROM foreign_tbl; DEBUG: starting remote transaction on connection 0x55ab0e416830 DEBUG: could not connect to postgres_fdw connection 0x55ab0e416830 DEBUG: closing connection 0x55ab0e416830 to reestablish a new one DEBUG: new postgres_fdw connection 0x55ab0e416830 for server "foreign_server" (user mapping oid 16407, userid 10) DEBUG: starting remote transaction on connection 0x55ab0e416830 DEBUG: closing remote transaction on connection 0x55ab0e416830 a1 | b1 -----+----- 100 | 200 Without the above message, it would look like we are starting remote txn, and closing connection without any reason. postgres=# SELECT * FROM foreign_tbl; DEBUG: starting remote transaction on connection 0x55ab0e4c0d50 DEBUG: closing connection 0x55ab0e4c0d50 to reestablish a new one DEBUG: new postgres_fdw connection 0x55ab0e4c0d50 for server "foreign_server" (user mapping oid 16389, userid 10) DEBUG: starting remote transaction on connection 0x55ab0e4c0d50 DEBUG: closing remote transaction on connection 0x55ab0e4c0d50 a1 | b1 -----+----- 100 | 200 With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
v5-Retry-Cached-Remote-Connections-For-postgres_fdw.patch
Description: Binary data