Re: PgFDW connection invalidation by ALTER SERVER/ALTER USER MAPPING

Thank you for the comments.

At Thu, 13 Jul 2017 16:54:42 +0530, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote in <CAFjFpRd0yz3v0rL2yxmr95e_iDntkeQia9709KXaHLyVcZ=_mQ(at)mail(dot)gmail(dot)com>
> On Thu, Jul 13, 2017 at 2:53 PM, Kyotaro HORIGUCHI
> <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> > 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>
> > 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?
> >
>
> The patch and the idea looks good to me. I haven't reviewed it
> thoroughly though.
>
> I noticed a type "suporious", I think you meant "spurious"? Probably

Right, it is too bad typo, but fixed it as "unnecessary", which
would more appropriate here.

> that comment should be part of the function which marks the connection
> as invalid e.g. InvalidateConnectionForMapping().

Agreed. It'd been there but somehow I moved it to there. I have
moved it back to the place it used to be.

> pgfdw_xact_callback() reports the reason for disconnection while
> closing a connection. May be we want to report the reason for
> disconnection here as well. Also, may be we want to create a function

Agreed. Also, I had placed LOG message there but removedxs. Now it
emits a DEBUG3 message as shown below.

| DEBUG: closing connection 0xxxx for option changes to take effect
| DEBUG: new postgres_fdw connection 0xxxx for server ".." (user mapping oid

> to discard connection from an entry so that we consistently do the
> same things while discarding a connection.

Sure. Now there's two places a connection is closed intentionally.

I'm a bit uneasy that many menbers of entry is getting reset in
so many places. Since the validity of an entry is checked only by
conn so it is enough to clear the flags of ConnCacheEntry only at
the time of connection creation. Instead,
pgfdw_reject_incomplete_xact_state_chanbe is no longer complains
on an inactive (conn == NULL) entry. I think this is safe but a
bit inconfident..

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center