Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away

On 2020/09/25 13:56, Bharath Rupireddy wrote:
> On Wed, Sep 23, 2020 at 8:19 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>>
>>> Please let me know if okay with the above agreed points, I will work on the new patch.
>>
>> Yes, please work on the patch! Thanks! I may revisit the above points later, though ;)
>>
>
> Thanks, attaching v4 patch. Please consider this for further review.

Thanks for updating the patch!

In the orignal code, disconnect_pg_server() is called when invalidation
occurs and connect_pg_server() is called when no valid connection exists.
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.
Basically I'd like to avoid duplicating disconnect_pg_server(),
connect_pg_server() and begin_remote_xact() if possible. Thought?

Your patch adds several codes into PG_CATCH() section, but it's better
to keep that section simple enough (as the source comment for
PG_CATCH() explains). So what about moving some codes out of PG_CATCH()
section?

+ 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?

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION