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

On 2020-12-30 17:59, Bharath Rupireddy wrote:
> On Wed, Dec 30, 2020 at 5:20 PM Alexey Kondratov
> <a.kondratov@postgrespro.ru> wrote:
>> 
>> On 2020-12-30 09:10, Bharath Rupireddy wrote:
>> I still have some doubts that it is worth of allowing to call
>> postgres_fdw_disconnect() in the explicit transaction block, since it
>> adds a lot of things to care and check for. But otherwise current 
>> logic
>> looks solid.
>> 
>> +                                errdetail("Such connections get 
>> closed either in the next use or
>> at the end of the current transaction.")
>> +                                : errdetail("Such connection gets 
>> closed either in the next use or
>> at the end of the current transaction.")));
>> 
>> Does it really have a chance to get closed on the next use? If foreign
>> server is dropped then user mapping should be dropped as well (either
>> with CASCADE or manually), but we do need user mapping for a local 
>> cache
>> lookup. That way, if I understand all the discussion up-thread
>> correctly, we can only close such connections at the end of xact, do 
>> we?
> 
> The next use of such a connection in the following query whose foreign
> server would have been dropped fails because of the cascading that can
> happen to drop the user mapping and the foreign table as well. During
> the start of the next query such connection will be marked as
> invalidated because xact_depth of that connection is > 1 and when the
> fail happens, txn gets aborted due to which pgfdw_xact_callback gets
> called and in that the connection gets closed. To make it more clear,
> please have a look at the scenarios [1].
> 

In my understanding 'connection gets closed either in the next use' 
means that connection will be closed next time someone will try to use 
it, i.e. GetConnection() will be called and it closes this connection 
because of a bad state. However, if foreign server is dropped 
GetConnection() cannot lookup the connection because it needs a user 
mapping oid as a key.

I had a look on your scenarios. IIUC, under **next use** you mean a 
select attempt from a table belonging to the same foreign server, which 
leads to a transaction abort and connection gets closed in the xact 
callback. Sorry, maybe I am missing something, but this just confirms 
that such connections only get closed in the xact callback (taking into 
account your recently committed patch [1]), so 'next use' looks 
misleading.

[1] 
https://www.postgresql.org/message-id/8b2aa1aa-c638-12a8-cb56-ea0f0a5019cf%40oss.nttdata.com


Regards
-- 
Alexey Kondratov

Postgres Professional https://www.postgrespro.com
Russian Postgres Company