On 2021/02/22 14:55, Bharath Rupireddy wrote:
> On Thu, Feb 4, 2021 at 9:36 AM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
>>
>> On Wed, Feb 3, 2021 at 4:22 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>>> Maybe my explanation in the previous email was unclear. What I think is; If the server-level option is explicitly
specified,its setting is used whatever GUC is. On the other hand, if the server-level option is NOT specified, GUC
settingis used. For example, if we define the server as follows, GUC setting is used because the server-level option is
NOTspecified.
>>>
>>> CREATE SERVER loopback FOREIGN DATA WRAPPER postgres;
>>>
>>> If we define the server as follows, the server-level setting is used.
>>>
>>> CREATE SERVER loopback FOREIGN DATA WRAPPER postgres OPTIONS (keep_connections 'on');
>>
>> Attaching v20 patch set. Now, server level option if provided
>> overrides the GUC.The GUC will be used only if server level option is
>> not provided. And also, both server level option and GUC are named the
>> same - "keep_connections".
>>
>> Please have a look.
>
> Attaching v21 patch set, rebased onto the latest master.
I agree to add the server-level option. But I'm still not sure if it's good idea to also expose that option as GUC.
Isn'tthe server-level option enough for most cases?
Also it's strange to expose only this option as GUC while there are other many postgres_fdw options?
With v21-002 patch, even when keep_connections GUC is disabled, the existing open connections are not close
immediately.Only connections used in the transaction are closed at the end of that transaction. That is, the existing
connectionsthat no transactions use will never be closed. I'm not sure if this behavior is intuitive for users.
Therefore for now I'm thinking to support the server-level option at first... Then if we find it's not enough for most
casesin practice, I'd like to consider to expose postgres_fdw options including keep_connections as GUC.
Thought?
BTW these patches fail to be applied to the master because of commit 27e1f14563. I updated and simplified the 003
patch.Patch attached.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION