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

On 2021/02/01 16:13, Bharath Rupireddy wrote:
> On Mon, Feb 1, 2021 at 12:29 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>> On 2021/01/30 9:28, Bharath Rupireddy wrote:
>>> On Sat, Jan 30, 2021 at 12:14 AM Fujii Masao
>>> <masao.fujii@oss.nttdata.com> wrote:
>>>> +                       /*
>>>> +                        * It doesn't make sense to show this entry in the output with a
>>>> +                        * NULL server_name as it will be closed at the xact end.
>>>> +                        */
>>>> +                       continue;
>>>>
>>>> -1 with this change because I still think that it's more useful to list
>>>> all the open connections.
>>>
>>> If postgres_fdw_get_connections doesn't have a "valid" column, then I
>>> thought it's better not showing server_name NULL in the output.
>>
>> Or if we don't have strong reason to remove "valid" column,
>> the current design is enough?
> 
> My only worry was that the statement from [1] "A cache flush should
> not cause user-visible state changes."

If we follow this strictly, I'm afraid that postgres_fdw_get_connections()
itself would also be a problem because the cached connections are affected
by cache flush and postgres_fdw_get_connections() shows that to users.
I'm not sure if removing "valid" column is actually helpful for that statement.

Anyway, for now we have the following options;

(1) keep the feature as it is
(2) remove "valid" column
         (2-1) show NULL for the connection whose server was dropped
    (2-2) show fixed value (e.g., <dropped>) for the connection whose server was dropped
(3) remove "valid" column and don't display connection whose server was dropped
(4) remove postgres_fdw_get_connections()

For now I like (1), but if others think "valid" column should be dropped,
I'm fine with (2). But I'd like to avoid (3) because I think that
postgres_fdw_get_connections() should list all the connections that
are actually being established. I have no strong opinion about whether
(2-1) or (2-2) is better, for now.

> But the newly added function
> postgres_fdw_get_connections is VOLATILE which means that the results
> returned by postgres_fdw_get_connections() is also VOLATILE. Isn't
> this enough, so that users will not get surprised with different
> results in case invalidations occur within the server by the time they
> run the query subsequent times and see different results than what
> they saw in the first run?

I'm not sure about this...

Regards,

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