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

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Cc: Zhihong Yu <zyu(at)yugabyte(dot)com>, Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>, "Hou, Zhijie" <houzj(dot)fnst(at)cn(dot)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PATCH] postgres_fdw connection caching - cause remote sessions linger till the local session exit
Date: 2021-02-01 07:39:48
Message-ID: CALj2ACW0vxkR-yU0+S-0yzGN0kZzA0_HVnsP5fgZVAANajwocQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Feb 1, 2021 at 12:43 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
> On 2021/01/27 10:06, Bharath Rupireddy wrote:
> > On Tue, Jan 26, 2021 at 8:38 AM Bharath Rupireddy
> > <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> >> I will post "keep_connections" GUC and "keep_connection" server level
> >> option patches later.
> >
> > Attaching v19 patch set for "keep_connections" GUC and
> > "keep_connection" server level option. Please review them further.
>
> These options are no longer necessary because we now support idle_session_timeout? If we want to disconnect the foreign server connections that sit on idle to prevent them from eating up the connection capacities in the foriegn servers, we can just set idle_session_timeout in those foreign servers. If we want to avoid the cluster-wide setting of idle_session_timeout, we can set that per role. One issue for this approach is that the connection entry remains even after idle_session_timeout happens. So postgres_fdw_get_connections() returns that connection even though it's actually closed by the timeout. Which is confusing. But which doesn't cause any actual problem, right? When the foreign table is accessed the next time, that connection entry is dropped, an error is detected, and then new connection will be remade.

First of all, idle_session_timeout is by default 0 i.e. disabled,
there are chances that users may not use that and don't want to set it
just for not caching any foreign server connection. A simple use case
where server level option can be useful is that, users are accessing
foreign tables (may be not that frequently, once in a while) from a
long running local session using foreign servers and they don't want
to keep the local session cache those connections, then setting this
server level option, keep_connections to false makes their life
easier, without having to depend on setting idle_session_timeout on
the remote server.

And, just using idle_session_timeout on a remote server may not help
us completely. Because the remote session may go away, while we are
still using that cached connection in an explicit txn on the local
session. Our connection retry will also not work because we are in the
middle of an xact, so the local explicit txn gets aborted.

So, IMO, we can still have both server level option as well as
postgres_fdw contrib level GUC (to tell the local session that "I
don't want to keep any foreign connections active" instead of setting
keep_connection server level option for each foreign server).

> Sorry I've not read the past long discussion about this feature. If there is the consensus that these options are still necessary and useful even when we have idle_session_timeout, please correct me.
>
> ISTM that it's intuitive (at least for me) to add this kind of option into the foreign server. But I'm not sure if it's good idea to expose the option as GUC. Also if there is the consensus about this, please correct me.

See here [1].

[1] - https://www.postgresql.org/message-id/f58d1df4ae58f6cf3bfa560f923462e0%40postgrespro.ru

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2021-02-01 08:04:25 Re: Add primary keys to system catalogs
Previous Message Peter Smith 2021-02-01 07:38:16 Re: Single transaction in the tablesync worker?