Re: dblink performance regression

Lists: pgsql-hackers
From: Joe Conway <mail(at)joeconway(dot)com>
To: "Hackers (PostgreSQL)" <pgsql-hackers(at)postgresql(dot)org>
Subject: dblink performance regression
Date: 2013-12-06 02:29:17
Message-ID: 52A1367D.7080505@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I was recently approached by someone with a dblink performance
regression, going back to somewhere between 8.3 (good) and 8.4 (bad).
They were measuring ~8% degradation in dblink connection speed. I was
able to replicate on my own hardware with the following script:

8<------------------------------
create or replace function test_dblink(loops int)
returns text as $$
declare
i int;
ret int;
begin
for i in 1..loops loop
IF i % 100 = 0 THEN
RAISE NOTICE 'connect attempt %', i;
END IF;
PERFORM dblink_connect('me','dbname=postgres host=w.x.y.z');
SELECT x into ret FROM dblink('me','SELECT 1', true) as x(x int);
PERFORM dblink_disconnect('me');
end loop;
return 'done';
end;
$$ language plpgsql;
\timing
select test_dblink(1000);
8<------------------------------

In my testing I saw a very consistent 4-5% degradation. I then did a
git bisect and traced the performance degradation to this commit:

- ------------------
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=e5de601267d98c5d60df6de8d436685c7105d149

committer Joe Conway <mail(at)joeconway(dot)com>
Tue, 9 Jun 2009 16:35:36 +0000 (16:35 +0000)
commit e5de601267d98c5d60df6de8d436685c7105d149

"Default client encoding to server encoding for dblink connections.
Addresses issue raised by Ruzsinszky Attila and confirmed by others."

- ------------------
with this diff:
- ------------------
http://git.postgresql.org/gitweb/?p=postgresql.git;a=blobdiff;f=contrib/dblink/dblink.c;h=e709ae9cc3b9f7177eaae08d11540a8590e7dc63;hp=283b4d0b25e9924dfc2d16075e7cfc1277ce5956;hb=e5de601267d98c5d60df6de8d436685c7105d149;hpb=adaf60131f81394939b5b55f74130343d520cd2d
- ------------------

Apparently setting client encoding is an expensive operation.

My proposed fix (diff attached) is to simply skip setting client
encoding in the case where it already matches server side encoding.
With the patch applied, and client encoding matching server (both
UTF8), the performance regression is completely gone.

Possibly additional work could go into determining if
PQsetClientEncoding() can be made more efficient, but I believe it
still makes sense in the case of dblink to do nothing when nothing is
required.

If there are no objections I'll commit this to all branches this weekend.

Thanks,

Joe

- --
Joe Conway
credativ LLC: http://www.credativ.us
Linux, PostgreSQL, and general Open Source
Training, Service, Consulting, & 24x7 Support
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJSoTZ9AAoJEDfy90M199hlVVsP/iG6XbcXYR2G2TC9I6FIaYzQ
c9503QcewPW+tY/0TAhzOYbXfdkkmRDf+4d9HYiCnYANhJW86dxmqtFKMyASbT74
BYjrZUHhafhSobDZBtM0t/6J1CMhLB/Gy66dziqkw0/hx0NeSuQqkluDazcIh/83
34hCRB2LNE7CXx88HvIc0hd6ACk7Ecrw7W6xyoznhmajHnq2G4Mp1AKXSkOTsYcJ
GkKy1G+u42M6pcBaNp6hnPuRj3HGECz6NdeuzWvb2IKLL6cY7C0fvCccqvkhcUpl
SPfIzlzxxtavzkkkxjOQUWrUFVWulZjeTFsQz3AWPQZoLtY+YDJ513R16Jh8s4RI
XoUWXRQVcMS2kWAIck6Nt/2zpIN9Rr4MlUgqh4mXlAZ59ErigVAqbhg9SAE0N4/h
Fa31OlTousTT4Wph34n/2nZK3uF46SiOHAoFipjRtGavbFW4lXKFryz6AEJ6e1Xy
fNpfNrXbKFkmRYdcafjZ7k6+NW70iCcH98A78wgyRLy59/b/M3u/K9TH5YN5tKLH
O+fK+S+s6eA9+omeu2hLl+6CkwDvEfBvzKkLu+9+sdV0s0b7VDt2vnMx/hg6E7EG
wCKB5X451lUz2MhXqX8vKiSGLk2ShmV8o8u0ovh4kFRV4YmjPK7LyDenIdmsoYuQ
NHiORCMc3V9USObJJ8so
=E/Hx
-----END PGP SIGNATURE-----

Attachment Content-Type Size
fix-dblink-encoding-regression-8.4.diff text/x-patch 941 bytes

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: "Hackers (PostgreSQL)" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dblink performance regression
Date: 2013-12-06 02:53:59
Message-ID: 27269.1386298439@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Joe Conway <mail(at)joeconway(dot)com> writes:
> Apparently setting client encoding is an expensive operation.

Yeah, it requires sending a command to the remote end. I'm not
sure if it'd be a good idea for PQsetClientEncoding to try to
skip the operation when the client encoding is already the
right thing. The given name might not be spelled canonically,
for one thing.

> My proposed fix (diff attached) is to simply skip setting client
> encoding in the case where it already matches server side encoding.

I have to think that we shouldn't need a string compare to figure out
if one integer encoding ID is equal to another. Why not just compare
PQclientEncoding(conn) to GetDatabaseEncoding()? Otherwise, seems
reasonable.

Actually ... there's an interesting conflict here. What if libpq hasn't
got the same encoding ID number assignments as the backend? I suspect
that the proposed code is actually wrong, or at least risks being wrong,
because it's far from clear whether the linker will resolve
pg_encoding_to_char() as being libpq's version or the backend's version.
The former choice would cause things to work, as long as the textual names
agree, but the latter choice would be just as broken as if we'd simply
compared the integer IDs.

I seem to remember that at some point we realized that the encoding ID
assignments are part of libpq's ABI and so can't practically be changed
ever, so the above may be moot. Even so, I think it's a bad idea to be
depending on pg_encoding_to_char() here, given the ambiguity in what it
references. It would be unsurprising to get build-time or run-time
failures on pickier platforms, as a consequence of that ambiguity. So
I'd still recommend comparing integer IDs as above, rather than this.

regards, tom lane


From: Joe Conway <mail(at)joeconway(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "Hackers (PostgreSQL)" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dblink performance regression
Date: 2013-12-06 03:05:14
Message-ID: 52A13EEA.4040708@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/05/2013 06:53 PM, Tom Lane wrote:
> I seem to remember that at some point we realized that the encoding
> ID assignments are part of libpq's ABI and so can't practically be
> changed ever, so the above may be moot. Even so, I think it's a
> bad idea to be depending on pg_encoding_to_char() here, given the
> ambiguity in what it references. It would be unsurprising to get
> build-time or run-time failures on pickier platforms, as a
> consequence of that ambiguity. So I'd still recommend comparing
> integer IDs as above, rather than this.

Great feedback as always -- thanks! Will make that change.

Joe

- --
Joe Conway
credativ LLC: http://www.credativ.us
Linux, PostgreSQL, and general Open Source
Training, Service, Consulting, & 24x7 Support
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJSoT7qAAoJEDfy90M199hlDqUP/jyyEQhBFBLK4AosiQnnRZuc
O55j4gFGIb9jIs1nRI+ndPon0gGW2LHWuxh7sUqnpkXSmRzn07A2t8Lj+5dqEX4+
1fz7Ty/2K60Ge7klZGz0OK9YuGZLlLMLUeJJmzJaGKy+zKOv51ceS1YuhPM/Y8a6
nI1dfMxJubbVV9rYzg+B0fLcC6ygTI2fIgurcUlq1/YgbwaX3ca+yOLfXhimnEWn
pAn4sqUznzn9uTKpxCIP2MkEslKRPWj94ocAtl5/eASA0FiRbAnQFw8Rwo8D9E/c
5WnRUUYYbPK1LfJHo8Qw++XnzBQdJc8/uY3aCbBMq7mFgha+ZqFnToTO5APmoU3C
xEOM1ZY7B6Y2y00hIUrmrFKrj6RRFq1/RNmvBridKwcH9W+jV+DsmgNVfwwd/2fZ
2toRxQ/cAEp+EAmlv/9mjc5KK6rSkCtGv3X5jSqOejBmIiZ+0UJm6JhduZyPTcEg
B/WfgM5UZ7O5QQCseDX80RPr9waAwL2aH/sjMSCXPNMH1pkSL1wda5Tl6DHvKEQ3
1T7F/moW8ne9Iece6uJjVBT33N8YEiUord8m3LTdCC5MWjr17hI4hV6Ixe0cVZXZ
97OwQtliLejSKg2mWOAmTFdDhJ6JmKmMOp/GtQk46ZbBwYWzD4fBsy5Pg2cayQtX
c+gK+aZA3WI7O4pgWIxx
=NXY7
-----END PGP SIGNATURE-----


From: Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Hackers (PostgreSQL)" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dblink performance regression
Date: 2013-12-06 03:16:38
Message-ID: CAFcNs+oXBN4AHXOimPuy_ndUwCtdMfywU+EuS9FPtEn2_8o0dg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, Dec 6, 2013 at 1:05 AM, Joe Conway <mail(at)joeconway(dot)com> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 12/05/2013 06:53 PM, Tom Lane wrote:
> > I seem to remember that at some point we realized that the encoding
> > ID assignments are part of libpq's ABI and so can't practically be
> > changed ever, so the above may be moot. Even so, I think it's a
> > bad idea to be depending on pg_encoding_to_char() here, given the
> > ambiguity in what it references. It would be unsurprising to get
> > build-time or run-time failures on pickier platforms, as a
> > consequence of that ambiguity. So I'd still recommend comparing
> > integer IDs as above, rather than this.
>
> Great feedback as always -- thanks! Will make that change.
>
>
Hi Joe, how are you?

Well, when Tom sent this email I was reviewing your patch and the main
suggestion is about use of 'pg_encoding_to_char' too... ;-)

The attached patch with my review!

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

Attachment Content-Type Size
fix-dblink-encoding-regression-8.4-v2.diff text/plain 1.6 KB

From: Joe Conway <mail(at)joeconway(dot)com>
To: fabriziomello(at)gmail(dot)com
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Hackers (PostgreSQL)" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dblink performance regression
Date: 2013-12-06 04:50:50
Message-ID: 52A157AA.7030606@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/05/2013 07:16 PM, Fabrízio de Royes Mello wrote:
> Hi Joe, how are you?

Hi Fabrizio -- great to hear from you! I'm well.

> Well, when Tom sent this email I was reviewing your patch and the
> main suggestion is about use of 'pg_encoding_to_char' too... ;-)
>
> The attached patch with my review!

Awesome, thanks for the review!

Joe

- --
Joe Conway
credativ LLC: http://www.credativ.us
Linux, PostgreSQL, and general Open Source
Training, Service, Consulting, & 24x7 Support
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJSoVeqAAoJEDfy90M199hlDqYQAKfYRX52AGHdd2eB/JT6hmXJ
TEYWENz65Cew9t0bPn2XKAHOkpAGf6JFT/XtloKCKwgF/nE0SIB1ETXiD7DVgIFU
EeMvyFnIkfg+wKehdhATQ6c7gpmQbNqs1DPZby7O5wQFyUFZB5Y7Tz7bUyZHPjTE
ml+GUkoulj3yYjC5Uf0q79k05karXOZS5V8jxjFig+nU2kE4wgpZ7BKUQXzAVr2C
7XCGdZXQ9fewE5CXKkiRJNsp9Si0PF0ahPeP7hZ/Jd4iWDibcv+ouhhStHZLrDY5
NjGR6BoaX/pASPU2lIGbkT/xEhp3ShMtn1FHnoYDqhcp3F5DraAJzS8Rr/eP9iZO
ks8aI0tYv2nlTwm+BiwRP+oTlSfQHlCyHN+3bMFTev3L6DmB/0kp/c/RPq4NYJB4
T2Khkq/+hFXXz01PK0l95O84m2o+zTpIefbbhY/xBrv+/+caDaTdJyID26G3ULJo
BmZv7jQlmUl53JBX5J4uHxhf8ChB7wEA8nkLfNuNnDDXUUjS0DL4oi8AX24X09w4
ac0KmK2yYI4R2FjrNvXNRvVnatEuAiGqbnldBO5YFLDmMKdI8Nq3EGf2ELphBfdZ
qDL2A+qvToJKj3iHYCQdnCZLmfH9wdxcDvZ6QgPJR/sRgHWvumCr3jxzNN219Lkw
DgQI8Dud1V4GCMx4dH6K
=VVw2
-----END PGP SIGNATURE-----


From: Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Hackers (PostgreSQL)" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dblink performance regression
Date: 2013-12-06 12:15:27
Message-ID: CAFcNs+rMQL_dSi4U4cKSCWWYVgxbHfzw3qjpf1A0uqb6MOyZ-w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, Dec 6, 2013 at 2:50 AM, Joe Conway <mail(at)joeconway(dot)com> wrote:
>
> On 12/05/2013 07:16 PM, Fabrízio de Royes Mello wrote:
> > Hi Joe, how are you?
>
> Hi Fabrizio -- great to hear from you! I'm well.
>

:-)

> > Well, when Tom sent this email I was reviewing your patch and the
> > main suggestion is about use of 'pg_encoding_to_char' too... ;-)
> >
> > The attached patch with my review!
>
> Awesome, thanks for the review!
>

You're welcome!

Greetings,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello


From: Joe Conway <mail(at)joeconway(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "Hackers (PostgreSQL)" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dblink performance regression
Date: 2013-12-08 01:07:37
Message-ID: 52A3C659.7020700@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/05/2013 07:05 PM, Joe Conway wrote:
> On 12/05/2013 06:53 PM, Tom Lane wrote:
>> I seem to remember that at some point we realized that the
>> encoding ID assignments are part of libpq's ABI and so can't
>> practically be changed ever, so the above may be moot. Even so,
>> I think it's a bad idea to be depending on pg_encoding_to_char()
>> here, given the ambiguity in what it references. It would be
>> unsurprising to get build-time or run-time failures on pickier
>> platforms, as a consequence of that ambiguity. So I'd still
>> recommend comparing integer IDs as above, rather than this.
>
> Great feedback as always -- thanks! Will make that change.

Committed to all active branches.

Joe

- --
Joe Conway
credativ LLC: http://www.credativ.us
Linux, PostgreSQL, and general Open Source
Training, Service, Consulting, & 24x7 Support
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJSo8ZZAAoJEDfy90M199hlCz4P/R9ngR28e3nPNOxpkKO8R2oE
t32gPhVP2fLhTstumolJHvkAQh8d+7em8aDT8lwGQ6a1DNGtwlJ2cXR64yjI9SXg
Vp4tJyuliZau7kitiDDtGXqbEyM+cCWJ8wFnckToERJtW2uXcC81kqGoac7lz1e9
o34UKizzD8PLA+N6TCG6GsOx8/W2kKEgru8up9jbUzuJDEmzUPkSn8rA2jHVl7oX
LKF0hPcFuNJept9A/iNZmBfPgXUrHL/4s9qJ0UQdzzcJDHZ3kalTgRKs9y035WXl
XQ/YbYry8/Qw5QgWz9g50/FDhK7jAP/ZBGU99lBGO9e4hxV2lTeeiz177hYzgckF
vqWNS/dKN3wtdtXmEwhwmyLodpJKJhLFWOsgfpRPPZZu5Ji+gwPYl3MQHY4THqMp
8kjzK/BL94C15NSCO5E5FBM3CGruYWPVzNZqS4PT+VqCupZ2+qxySgL/0riMbx+J
GmBF/C9EGQzrRq7idz8O7/7Frb9TwjBgj+I0wns5NKF1vJpJ2fbzafmvpLkgJ/oI
bGhJt43D645BgnAKrInIq6mMbsYAr10pK6v03gJhiJUZREeu9wX5pVIzFV1R1PgC
ZS7evjftSP5MwdGs8geEVcQKbx42jW+kQos/HKH6saLKKf5s+cdXNiXaCOj/8cI7
1TZWt31Z9PqS7FuSDkXR
=AWVh
-----END PGP SIGNATURE-----


From: Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Hackers (PostgreSQL)" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dblink performance regression
Date: 2013-12-08 01:16:33
Message-ID: CAFcNs+pWFLKrxi3Mm0dEuvcFc_fPKVJEi3YgGBdY8CG5FGamhA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Sat, Dec 7, 2013 at 11:07 PM, Joe Conway <mail(at)joeconway(dot)com> wrote:
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 12/05/2013 07:05 PM, Joe Conway wrote:
> > On 12/05/2013 06:53 PM, Tom Lane wrote:
> >> I seem to remember that at some point we realized that the
> >> encoding ID assignments are part of libpq's ABI and so can't
> >> practically be changed ever, so the above may be moot. Even so,
> >> I think it's a bad idea to be depending on pg_encoding_to_char()
> >> here, given the ambiguity in what it references. It would be
> >> unsurprising to get build-time or run-time failures on pickier
> >> platforms, as a consequence of that ambiguity. So I'd still
> >> recommend comparing integer IDs as above, rather than this.
> >
> > Great feedback as always -- thanks! Will make that change.
>
> Committed to all active branches.
>

IMHO is more elegant create a procedure to encapsulate the code to avoid
redundancy.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello


From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: fabriziomello(at)gmail(dot)com
Cc: Joe Conway <mail(at)joeconway(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Hackers (PostgreSQL)" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dblink performance regression
Date: 2013-12-08 01:20:08
Message-ID: CAB7nPqQq67sdN0x4OgvZFcaODZ-ans=4xtLY+8s8QR+6bvK6xQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Sun, Dec 8, 2013 at 10:16 AM, Fabrízio de Royes Mello
<fabriziomello(at)gmail(dot)com> wrote:
>
>
>
> On Sat, Dec 7, 2013 at 11:07 PM, Joe Conway <mail(at)joeconway(dot)com> wrote:
>>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> On 12/05/2013 07:05 PM, Joe Conway wrote:
>> > On 12/05/2013 06:53 PM, Tom Lane wrote:
>> >> I seem to remember that at some point we realized that the
>> >> encoding ID assignments are part of libpq's ABI and so can't
>> >> practically be changed ever, so the above may be moot. Even so,
>> >> I think it's a bad idea to be depending on pg_encoding_to_char()
>> >> here, given the ambiguity in what it references. It would be
>> >> unsurprising to get build-time or run-time failures on pickier
>> >> platforms, as a consequence of that ambiguity. So I'd still
>> >> recommend comparing integer IDs as above, rather than this.
>> >
>> > Great feedback as always -- thanks! Will make that change.
>>
>> Committed to all active branches.
>>
>
> IMHO is more elegant create a procedure to encapsulate the code to avoid
> redundancy.
Yep, perhaps something like PQsetClientEncodingIfDifferent or similar
would make sense.
--
Michael


From: Josh Berkus <josh(at)agliodbs(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: dblink performance regression
Date: 2013-12-08 01:25:45
Message-ID: 52A3CA99.3090804@agliodbs.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

All,

I tested out Joe's original patch, and it does eliminate the 8%
performance regression.

Will try the new one.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


From: Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Joe Conway <mail(at)joeconway(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Hackers (PostgreSQL)" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dblink performance regression
Date: 2013-12-08 01:41:56
Message-ID: CAFcNs+qJkM72yJiGP+Ti6S2xdx--VYt2nG=2F7Sd6YowykwCrg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Sat, Dec 7, 2013 at 11:20 PM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:
> >
> > IMHO is more elegant create a procedure to encapsulate the code to avoid
> > redundancy.
> Yep, perhaps something like PQsetClientEncodingIfDifferent or similar
> would make sense.
>

Well I think at this first moment we can just create a procedure inside the
dblink contrib and not touch in libpq.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello


From: Joe Conway <mail(at)joeconway(dot)com>
To: fabriziomello(at)gmail(dot)com, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Hackers (PostgreSQL)" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dblink performance regression
Date: 2013-12-08 01:50:31
Message-ID: 52A3D067.4050801@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/07/2013 05:41 PM, Fabrízio de Royes Mello wrote:
>
> On Sat, Dec 7, 2013 at 11:20 PM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com <mailto:michael(dot)paquier(at)gmail(dot)com>>
> wrote:
>>>
>>> IMHO is more elegant create a procedure to encapsulate the code
>>> to avoid redundancy.
>> Yep, perhaps something like PQsetClientEncodingIfDifferent or
>> similar would make sense.
>
> Well I think at this first moment we can just create a procedure
> inside the dblink contrib and not touch in libpq.

Maybe a libpq function could be done for 9.4, but not for back branches.

I don't think it makes sense to create a new function in dblink either
- -- we're only talking about two lines of added redundancy which is
less lines of code than a new function would add. But if we create
PQsetClientEncodingIfDifferent() (or whatever) we can remove those
extra lines in 9.4 ;-)

Joe

- --
Joe Conway
credativ LLC: http://www.credativ.us
Linux, PostgreSQL, and general Open Source
Training, Service, Consulting, & 24x7 Support
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJSo9BnAAoJEDfy90M199hluqYP/RyoKh9nvC49H3xDl4wBLh4n
0OQ5nKJk8RkQ5d0MLbgn9t1xiQ+RltMcHQQEDoPlrn388DNTqOP31TqCHtI11S5l
ZOjZw6eKvcp0KEzk723kZCq9d2N1uRb95K2z5jFUXbeZ2pO6yj8ohdnh8J9i1VQE
iI5F76yeUJCO8YRmHMJs39Fy3Qtq0dsXesPYBRuEJqHy7cGh9hpYHPuqHFyW19Kg
0q1nPMa7TYpKRECa1wi+Gt2BJqd70AdnOZZipqqCR2bMJqmcpBy3jo94vedaarAz
Wtunn4mk0/2LCNsAjgAdA33FYNfRpgL2c99IQ1DK5hwW9mdH2WH6G+8Eaf5zGhps
ZWXJRQSYjfUCmMOoGudEKNX3H3prrNpqltQuC978i4ddKK/78yX1wJD10I8qVNHy
MRlSoTFfomVYPW0054Jk6LR1f/RKGD4yuiIPDv8dI/gWINT1HveBGkvSf9wnY578
wjh2iLJ790o4CNK/334ooggPnlbBS4yQ+e+xsDcaJ2pc1cWJr/gCUf3cliUtv+rI
MnFvsbq4vEjhBE3Tr6LYPwivCzKpiEz2L0/oO8sShHhm/dfr9UGPUyeDO43phP+m
NFQXoh6oCuleBXk/N874yAp9EDXtu3g9E1PUMMsbplXjiH6mLp8OWmRbvQ9Qw3zu
BFtOonWViuPz5ILObc3i
=o0XG
-----END PGP SIGNATURE-----


From: Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Joe Conway <mail(at)joeconway(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Hackers (PostgreSQL)" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dblink performance regression
Date: 2013-12-08 01:52:15
Message-ID: CAFcNs+rZLMmDAAdH0RE5JTMtTu1TdNkNT9Zc9fEoYzg4f3CmoA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Sat, Dec 7, 2013 at 11:41 PM, Fabrízio de Royes Mello <
fabriziomello(at)gmail(dot)com> wrote:
>
>
> On Sat, Dec 7, 2013 at 11:20 PM, Michael Paquier <
michael(dot)paquier(at)gmail(dot)com> wrote:
> > >
> > > IMHO is more elegant create a procedure to encapsulate the code to
avoid
> > > redundancy.
> > Yep, perhaps something like PQsetClientEncodingIfDifferent or similar
> > would make sense.
> >
>
> Well I think at this first moment we can just create a procedure inside
the dblink contrib and not touch in libpq.
>

Something like the attached.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

Attachment Content-Type Size
dblink_set_client_encoding_v1.patch text/x-diff 1.8 KB

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: "fabriziomello(at)gmail(dot)com" <fabriziomello(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Hackers (PostgreSQL)" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dblink performance regression
Date: 2013-12-08 02:15:57
Message-ID: AA6649D2-A0EA-4620-8263-59C959CFDA2D@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


> On 2013/12/08, at 10:50, Joe Conway <mail(at)joeconway(dot)com> wrote:
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>> On 12/07/2013 05:41 PM, Fabrízio de Royes Mello wrote:
>>
>> On Sat, Dec 7, 2013 at 11:20 PM, Michael Paquier
>> <michael(dot)paquier(at)gmail(dot)com <mailto:michael(dot)paquier(at)gmail(dot)com>>
>> wrote:
>>>>
>>>> IMHO is more elegant create a procedure to encapsulate the code
>>>> to avoid redundancy.
>>> Yep, perhaps something like PQsetClientEncodingIfDifferent or
>>> similar would make sense.
>>
>> Well I think at this first moment we can just create a procedure
>> inside the dblink contrib and not touch in libpq.
>
> Maybe a libpq function could be done for 9.4, but not for back branches.
Agreed as this would change the spec of libpq between minor releases. Sorry for not being clear myself.

> -- we're only talking about two lines of added redundancy which is
> less lines of code than a new function would add. But if we create
> PQsetClientEncodingIfDifferent() (or whatever) we can remove those
> extra lines in 9.4 ;-)

--
Michael
(Sent from my mobile phone)


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: fabriziomello(at)gmail(dot)com, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, "Hackers (PostgreSQL)" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dblink performance regression
Date: 2013-12-08 18:12:11
Message-ID: 14354.1386526331@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Joe Conway <mail(at)joeconway(dot)com> writes:
> I don't think it makes sense to create a new function in dblink either
> -- we're only talking about two lines of added redundancy which is
> less lines of code than a new function would add.

Indeed, and I think the claim that such a function "encapsulates" anything
useful is pretty silly as well. I think the committed patch is fine.

regards, tom lane


From: Jim Nasby <jim(at)nasby(dot)net>
To: Joe Conway <mail(at)joeconway(dot)com>, fabriziomello(at)gmail(dot)com, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Hackers (PostgreSQL)" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dblink performance regression
Date: 2013-12-10 01:11:49
Message-ID: 52A66A55.5010300@nasby.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 12/7/13 7:50 PM, Joe Conway wrote:
> On 12/07/2013 05:41 PM, Fabrízio de Royes Mello wrote:
>> >
>> >On Sat, Dec 7, 2013 at 11:20 PM, Michael Paquier
>> ><michael(dot)paquier(at)gmail(dot)com <mailto:michael(dot)paquier(at)gmail(dot)com>>
>> >wrote:
>>>> >>>
>>>> >>>IMHO is more elegant create a procedure to encapsulate the code
>>>> >>>to avoid redundancy.
>>> >>Yep, perhaps something like PQsetClientEncodingIfDifferent or
>>> >>similar would make sense.
>> >
>> >Well I think at this first moment we can just create a procedure
>> >inside the dblink contrib and not touch in libpq.
> Maybe a libpq function could be done for 9.4, but not for back branches.

Stupid question... why don't we just pass encoding in with the other connection parameters? That eliminates any ambiguity. The only issue would be if the user also passed that in via connstr... though now that I think about it, we currently silently ignore that parameter, which IMHO is bad. We should either respect if the user passes that in (ie: not do anything at all), or we should throw an error.
--
Jim C. Nasby, Data Architect jim(at)nasby(dot)net
512.569.9461 (cell) http://jim.nasby.net


From: Josh Berkus <josh(at)agliodbs(dot)com>
To: Joe Conway <mail(at)joeconway(dot)com>, fabriziomello(at)gmail(dot)com, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Hackers (PostgreSQL)" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dblink performance regression
Date: 2014-01-16 19:30:51
Message-ID: 52D8336B.1080501@agliodbs.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 12/07/2013 05:50 PM, Joe Conway wrote:
> On 12/07/2013 05:41 PM, Fabrízio de Royes Mello wrote:
>
>> On Sat, Dec 7, 2013 at 11:20 PM, Michael Paquier
>> <michael(dot)paquier(at)gmail(dot)com <mailto:michael(dot)paquier(at)gmail(dot)com>>
>> wrote:
>>>>
>>>> IMHO is more elegant create a procedure to encapsulate the code
>>>> to avoid redundancy.
>>> Yep, perhaps something like PQsetClientEncodingIfDifferent or
>>> similar would make sense.
>
>> Well I think at this first moment we can just create a procedure
>> inside the dblink contrib and not touch in libpq.
>
> Maybe a libpq function could be done for 9.4, but not for back branches.
>
> I don't think it makes sense to create a new function in dblink either
> -- we're only talking about two lines of added redundancy which is
> less lines of code than a new function would add. But if we create
> PQsetClientEncodingIfDifferent() (or whatever) we can remove those
> extra lines in 9.4 ;-)

Hey, since we're about to do 9.3.3: was this patch ever committed?

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


From: Joe Conway <mail(at)joeconway(dot)com>
To: Josh Berkus <josh(at)agliodbs(dot)com>, <fabriziomello(at)gmail(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Hackers (PostgreSQL)" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dblink performance regression
Date: 2014-01-16 20:44:26
Message-ID: 1439ccc3040.274a.4c0943eccb4f26345b421b6bb9592ac9@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

yes

Joe

Sent with AquaMail for Android
http://www.aqua-mail.com

On January 16, 2014 2:32:55 PM Josh Berkus <josh(at)agliodbs(dot)com> wrote:

> On 12/07/2013 05:50 PM, Joe Conway wrote:
> > On 12/07/2013 05:41 PM, Fabrízio de Royes Mello wrote:
> > >> On Sat, Dec 7, 2013 at 11:20 PM, Michael Paquier >>
> <michael(dot)paquier(at)gmail(dot)com <mailto:michael(dot)paquier(at)gmail(dot)com>>
> >> wrote:
> >>>>
> >>>> IMHO is more elegant create a procedure to encapsulate the code
> >>>> to avoid redundancy.
> >>> Yep, perhaps something like PQsetClientEncodingIfDifferent or
> >>> similar would make sense.
> > >> Well I think at this first moment we can just create a procedure
> >> inside the dblink contrib and not touch in libpq.
> > Maybe a libpq function could be done for 9.4, but not for back branches.
> > I don't think it makes sense to create a new function in dblink either
> > -- we're only talking about two lines of added redundancy which is
> > less lines of code than a new function would add. But if we create
> > PQsetClientEncodingIfDifferent() (or whatever) we can remove those
> > extra lines in 9.4 ;-)
>
> Hey, since we're about to do 9.3.3: was this patch ever committed?
>
>
> --
> Josh Berkus
> PostgreSQL Experts Inc.
> http://pgexperts.com