Re: [HACKERS] Using user mapping OID as hash key for connection hash

[Robert Haas]

On Wed, Jan 27, 2016 at 6:32 AM, Ashutosh Bapat
 wrote:
> As discussed in postgres_fdw join pushdown thread [1], for two different
> effective local users which use public user mapping we will be creating two
> different connections to the foreign server with the same credentials.
>
> Robert suggested [2] that we should use user mapping OID as the connection
> cache key instead of current userid and serverid.
>
> There are two patches attached here:
> 1. pg_fdw_concache.patch.short - shorter version of the fix. Right now
> ForeignTable, ForeignServer have corresponding OIDs saved in these
> structures. But UserMapping doesn't. Patch adds user mapping OID as a member
> to this structure. This member is then used as key in GetConnection().
> 2. pg_fdw_concache.patch.large - most of the callers of GetConnection() get
> ForeignServer object just to pass it to GetConnection(). GetConnection can
> obtain ForeignServer by using serverid from UserMapping and doesn't need
> ForeignServer to be an argument. Larger version of patch has this change.

The long form seems like a good idea, so I committed that one.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Using user mapping OID as hash key for connection hash

[Ashutosh Bapat]

Hi All,
As discussed in postgres_fdw join pushdown thread [1], for two different
effective local users which use public user mapping we will be creating two
different connections to the foreign server with the same credentials.

Robert suggested [2] that we should use user mapping OID as the connection
cache key instead of current userid and serverid.

There are two patches attached here:
1. pg_fdw_concache.patch.short - shorter version of the fix. Right now
ForeignTable, ForeignServer have corresponding OIDs saved in these
structures. But UserMapping doesn't. Patch adds user mapping OID as a
member to this structure. This member is then used as key in
GetConnection().
2. pg_fdw_concache.patch.large - most of the callers of GetConnection() get
ForeignServer object just to pass it to GetConnection(). GetConnection can
obtain ForeignServer by using serverid from UserMapping and doesn't need
ForeignServer to be an argument. Larger version of patch has this change.

GetConnection has named the UserMapping argument as just "user", ideally it
should have been named user_mapping. But that seems to be too obvious to be
unintentional. So, I have left that change.

The patch has added userid and user mapping oid to a debug3 message in
GetConnection(). the message is displayed when a new connection to foreign
server is created. With only that change, if we run script multi_conn.sql
(attached) we see that it's creating two connections when same user mapping
is used. Also attached is the output of the same script run on my setup.
Since min_messages is set to DEBUG3 there are too many unrelated messages.
So, search for "new postgres_fdw connection .." for new connection messages.

I have included the changes to the DEBUG3 message in GetConnection(), since
it may be worth committing those changes. In case, reviewers/committers
disagree, those chagnes can be removed.

[1]
http://www.postgresql.org/message-id/CAFjFpRf-LiD5bai4D6cSUseJh=xxjqipo_vn8mtnzg16tmw...@mail.gmail.com
[2]
http://www.postgresql.org/message-id/ca+tgmoymmv_du-vppq1d7ufsjaopbq+lgpxtchnuqfobjg2...@mail.gmail.com
-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


pg_fdw_concache.patch.large
Description: Binary data


pg_fdw_concache.patch.short
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers