On Fri, Sep 25, 2020 at 3:21 PM Fujii Masao <
masao.fujii@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 0x55ab0e416830DEBUG: 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
