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

On 2021/01/29 14:53, Bharath Rupireddy wrote:
> On Fri, Jan 29, 2021 at 10:55 AM Fujii Masao
> <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>>>> BTW, even if we change pgfdw_inval_callback() so that it doesn't close
>>>> the connection at all, ISTM that the results of postgres_fdw_get_connections()
>>>> would not be stable because entry->invalidated would vary based on
>>>> whether CLOBBER_CACHE_ALWAYS is used or not.
>>>
>>> Yes, after the above change (removing disconnect_pg_server in
>>> pgfdw_inval_callback), our tests don't get stable because
>>> postgres_fdw_get_connections shows the valid state of the connections.
>>> I think we can change postgres_fdw_get_connections so that it only
>>> shows the active connections server name but not valid state. Because,
>>> the valid state is something dependent on the internal state change
>>> and is not consistent with the user expectation but we are exposing it
>>> to the user. Thoughts?
>>
>> I don't think that's enough because even the following simple
>> queries return the different results, depending on whether
>> CLOBBER_CACHE_ALWAYS is used or not.
>>
>> SELECT * FROM ft6; -- ft6 is the foreign table
>> SELECT server_name FROM postgres_fdw_get_connections();
>>
>> When CLOBBER_CACHE_ALWAYS is used, postgres_fdw_get_connections()
>> returns no records because the connection is marked as invalidated,
>> and then closed at xact callback in SELECT query. Otherwise,
>> postgres_fdw_get_connections() returns at least one connection that
>> was established in the SELECT query.
>
> Right. In that case, after changing postgres_fdw_get_connections() so
> that it doesn't output the valid state of the connections at all, we

You're thinking to get rid of "valid" column? Or hide it from the test query
(e.g., SELECT server_name from postgres_fdw_get_connections())?

> can have all the new function test cases inside an explicit txn block.
> So even if the clobber cache invalidates the connections, they don't
> get closed until the end of main xact, the tests will be stable.
> Thoughts?

Also if there are cached connections before starting that transaction,
they should be closed or established again before executing
postgres_fdw_get_connections(). Otherwise, those connections are
returned from postgres_fdw_get_connections() when
CLOBBER_CACHE_ALWAYS is not used, but not when it's used.

Regards,

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