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

On 2020/12/15 1:40, Bharath Rupireddy wrote:
> On Mon, Dec 14, 2020, 9:47 PM Bharath Rupireddy
> <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>>
>> On Mon, Dec 14, 2020 at 8:03 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>>>> We will not output if the invalidated entry has no active connection[2], so if we fix the connection leak issue with the above discussed fix i.e closing all the invalidated connections at the end of next xact, there are less chances that we will output invalidated entries in the postgres_fdw_get_connections() output. Only case we may show up invalidated connections(which have active connections entry->conn) in the postgres_fdw_get_connections() output is as follows:
>>>>
>>>> 1) say we have few cached active connections exists in session 1
>>>> 2) drop the user mapping (in another session) associated with any of the cached connections to make that entry invalid
>>>> 3) run select * from postgres_fdw_get_connections(); in session 1. At the start of the xact, the invalidation message gets processed and the corresponding entry gets marked as invalid. If we allow invalid connections (that have entry->conn) to show up in the output, then we show them in the result of the query. At the end of xact, we close these invalid connections, in this case, user might think that he still have invalid connections active.
>>>
>>> What about the case where the transaction started at the above 1) at session 1, and postgres_fdw_get_connections() in the above 3) is called within that transaction at session 1? In this case, postgres_fdw_get_connections() can return even invalidated connections?
>>
>> In that case, since the user mapping would have been dropped in
>> another session and we are in the middle of a txn in session 1, the
>> entries would not get marked as invalid until the invalidation message
>> gets processed by the session 1 which may happen if the session 1

Yes, and this can happen by other commands, for example, CREATE TABLE.

>> opens a sub txn, if not then for postgres_fdw_get_connections() the
>> entries will still be active as they would not have been marked as
>> invalid yet and postgres_fdw_get_connections() would return them in
>> the output.
>
> One more point for the above scenario: if the user mapping is dropped
> in another session, then cache lookup for that entry in the
> postgres_fdw_get_connections() returns a null tuple which I plan to
> not throw an error, but just to skip in that case and continue. But if
> the user mapping is not dropped in another session but altered, then
> postgres_fdw_get_connections() still can show that in the output.

Yes, so *if* we really want to return even connection invalidated by drop of
user mapping, the cached connection entry may need to store not only
user mapping id but also server id so that we can get the server name without
user mapping entry.

Regards,

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