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

On Fri, Jul 14, 2017 at 2:04 PM, Kyotaro HORIGUCHI
<horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> 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.

Thanks. Can you please add this to the next CF. I don't think we will
be able to accept this change in v10.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company