PgFDW connection invalidation by ALTER SERVER/ALTER USER MAPPING

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: pgsql-hackers(at)postgresql(dot)org
Cc: Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp, fcs1(at)poczta(dot)onet(dot)pl, tgl(at)sss(dot)pgh(dot)pa(dot)us
Subject: PgFDW connection invalidation by ALTER SERVER/ALTER USER MAPPING
Date: 2017-07-13 09:23:00
Message-ID: 20170713.182300.148193868.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

Hello, moved to pgsql-hackers.

This is the revased and revised version of the previous patch.

At Thu, 13 Jul 2017 13:42:49 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote in <20170713(dot)134249(dot)97825982(dot)horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
> At Tue, 11 Jul 2017 15:39:14 -0400, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in <6234(dot)1499801954(at)sss(dot)pgh(dot)pa(dot)us>
> > Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> writes:
> > > Horiguchi-san,
> > > On 2017/07/11 10:28, Kyotaro HORIGUCHI wrote:
> > >> I faintly recall such discussion was held aroud that time and
> > >> maybe we concluded that we don't do that but I haven't find such
> > >> a thread in pgsql-hackers..
> >
> > > I mentioned it in my reply. Here again:
> > > https://www.postgresql.org/message-id/20160405.184408.166437663.horiguchi.kyotaro%40lab.ntt.co.jp
> >
> > The followup discussion noted that that approach was no good because it
> > would only close connections in the same session that had done the ALTER
> > SERVER. I think the basic idea of marking postgres_fdw connections as
> > needing to be remade when next possible is OK, but we have to drive it
> > off catcache invalidation events, the same as we did in c52d37c8b. An
> > advantage of that way is we don't need any new hooks in the core code.
> >
> > Kyotaro-san, are you planning to update your old patch?
>
> I'm pleased to do that. I will reconsider the way shown in a mail
> in the thread soon.

done.

(As a recap) Changing of some options such as host of a foreign
server or password of a user mapping make the existing
connections stale, but postgres_fdw continue using them.

The attached patch does the following things.

- postgres_fdw registers two invalidation callbacks on loading.

- On any change on a foreign server or a user mapping, the
callbacks mark the affected connection as 'invalid'

- The invalidated connections will be once disconnected before
the next execution if no transaction exists.

As the consequence, changes of options properly affects
subsequent queries in the next transaction on any session. It
occurs after whatever option has been modifed.

======
create server sv1 foreign data wrapper postgres_fdw options (host '/tmp', port '5432', dbname 'postgres');
create user mapping for public server sv1;
create table t (a int);
create foreign table ft1 (a int) server sv1 options (table_name 't1');
begin;
select * from ft1; -- no error
alter server sv1 options (set host '/tmpe');
select * from ft1; -- no error because transaction is living.
commit;
select * from ft1;
ERROR: could not connect to server "sv1" - NEW
======

This patch is postgres_fdw-private but it's annoying that hash
value of syscache is handled in connection.c. If we allow to add
something to the core for this feature, I could add a new member
in FdwRoutine to notify invalidation of mapping and server by
oid. (Of course it is not back-patcheable, though)

Does anyone have opinitons or suggestions?

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
pgfdw_reconnect_on_option_change_v1.patch text/x-patch 5.8 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message jothiprasath216 2017-07-13 10:04:42 Re: BUG #14736: Crash on postgresql server by autovacuum worker process
Previous Message Tom Lane 2017-07-13 06:36:29 Re: Re: BUG #14736: Crash on postgresql server by autovacuum worker process

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2017-07-13 09:34:58 Re: New partitioning - some feedback
Previous Message Tatsuo Ishii 2017-07-13 08:35:05 Re: SCRAM auth and Pgpool-II