Re: [PATCH] postgres_fdw connection caching - cause remote sessions linger till the local session exit

On Mon, Jan 18, 2021 at 11:58 AM Fujii Masao
<masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>
>
>
> On 2021/01/18 15:02, Hou, Zhijie wrote:
> >> We need to create the loopback3 with user mapping public, otherwise the
> >> test might become unstable as shown below. Note that loopback and
> >> loopback2 are not dropped in the test, so no problem with them.
> >>
> >> ALTER SERVER loopback OPTIONS (ADD use_remote_estimate 'off'); DROP
> >> SERVER loopback3 CASCADE;
> >> NOTICE: drop cascades to 2 other objects
> >> -DETAIL: drop cascades to user mapping for postgres on server loopback3
> >> +DETAIL: drop cascades to user mapping for bharath on server loopback3
> >>
> >> Attaching v2 patch for postgres_fdw_get_connections. Please have a look.
> > Hi
> >
> > I have a comment for the doc about postgres_fdw_get_connections.
> >
> > + <term><function>postgres_fdw_get_connections(OUT server_name text, OUT valid boolean) returns setof record</function></term>
> > + <listitem>
> > + <para>
> > + This function returns the foreign server names of all the open
> > + connections that <filename>postgres_fdw</filename> established from
> > + the local session to the foreign servers. It also returns whether
> > + each connection is valid or not. <literal>false</literal> is returned
> > + if the foreign server connection is used in the current local
> > + transaction but its foreign server or user mapping is changed or
> > + dropped, and then such invalid connection will be closed at
> > + the end of that transaction. <literal>true</literal> is returned
> > + otherwise. If there are no open connections, no record is returned.
> > + Example usage of the function:
> >
> > The doc seems does not memtion the case when the function returns NULL in server_name.
> > Users may be a little confused about why NULL was returned.
>
> Yes, so what about adding
>
> (Note that the returned server name of invalid connection is NULL if its server is dropped)
>
> into the following (just after "dropped")?
>
> + if the foreign server connection is used in the current local
> + transaction but its foreign server or user mapping is changed or
> + dropped
>
> Or better description?

+1 to add it after "dropped (Note ........)", how about as follows
with slight changes?

dropped (Note that server name of an invalid connection can be NULL if
the server is dropped), and then such .....

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com