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

On 2021/01/18 15:37, Bharath Rupireddy wrote:
> On Mon, Jan 18, 2021 at 11:58 AM Fujii Masao
> <masao.fujii@oss.nttdata.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 .....

Yes, I like this one. One question is; "should" or "is" is better than
"can" in this case because the server name of invalid connection is
always NULL when its server is dropped?

Regards,

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