Re: postgres_fdw - cached connection leaks if the associated user mapping/foreign server is dropped

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: "Hou, Zhijie" <houzj(dot)fnst(at)cn(dot)fujitsu(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Cc: Zhihong Yu <zyu(at)yugabyte(dot)com>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Subject: Re: postgres_fdw - cached connection leaks if the associated user mapping/foreign server is dropped
Date: 2020-12-18 13:16:48
Message-ID: CALj2ACXoBUnEExtbrLzXfwZR_Czf1f2ATdPffZU-4JE1K-J9sw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Dec 18, 2020 at 6:39 PM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> On Fri, Dec 18, 2020 at 5:06 PM Hou, Zhijie <houzj(dot)fnst(at)cn(dot)fujitsu(dot)com> wrote:
> > I have an issue about the existing testcase.
> >
> > """
> > -- Test that alteration of server options causes reconnection
> > SELECT c3, c4 FROM ft1 ORDER BY c3, c1 LIMIT 1; -- should work
> > ALTER SERVER loopback OPTIONS (SET dbname 'no such database');
> > SELECT c3, c4 FROM ft1 ORDER BY c3, c1 LIMIT 1; -- should fail
> > DO $d$
> > BEGIN
> > EXECUTE $$ALTER SERVER loopback
> > OPTIONS (SET dbname '$$||current_database()||$$')$$;
> > END;
> > $d$;
> > SELECT c3, c4 FROM ft1 ORDER BY c3, c1 LIMIT 1; -- should work again
> > """
> >
> > IMO, the above case is designed to test the following code[1]:
> > With the patch, it seems the following code[1] will not work for this case, right?
> > (It seems the connection will be disconnect in pgfdw_xact_callback)
> >
> > I do not know does it matter, or should we add a testcase to cover that?
> >
> > [1] /*
> > * If the connection needs to be remade due to invalidation, disconnect as
> > * soon as we're out of all transactions.
> > */
> > if (entry->conn != NULL && entry->invalidated && entry->xact_depth == 0)
> > {
> > elog(DEBUG3, "closing connection %p for option changes to take effect",
> > entry->conn);
> > disconnect_pg_server(entry);
> > }
>
> Yes you are right. With the patch if the server is altered in the same
> session in which the connection exists, the connection gets closed at
> the end of that alter query txn, not at the beginning of the next
> foreign tbl query. So, that part of the code in GetConnection()
> doesn't get hit. Having said that, this code gets hit when the alter
> query is run in another session and the connection in the current
> session gets disconnected at the start of the next foreign tbl query.
>
> If we want to cover it with a test case, we might have to alter the
> foreign server in another session. I'm not sure whether we can open
> another session from the current psql session and run a sql command.
>
> With Regards,
> Bharath Rupireddy.
> EnterpriseDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2020-12-18 13:18:53 Re: Proposed patch for key managment
Previous Message Stephen Frost 2020-12-18 13:12:21 Re: Proposed patch for key managment