On Tue, Mar 19, 2013 at 6:10 PM, Daniel Farina <dan...@heroku.com> wrote: > On Tue, Mar 19, 2013 at 3:06 PM, Daniel Farina <dan...@heroku.com> wrote: >> On Tue, Mar 19, 2013 at 2:41 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: >>> Daniel Farina <dan...@heroku.com> writes: >>>> On Tue, Mar 19, 2013 at 1:16 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: >>>>> I'd be inclined to eat the cost of calling PQparameterStatus every time >>>>> (which won't be that much) and instead try to optimize by avoiding the >>>>> GUC-setting overhead if the current value matches the local setting. >>>>> But even that might be premature optimization. Did you do any >>>>> performance testing to see if there was a problem worth avoiding? >>> >>>> Nope; should I invent a new way to do that, or would it be up to >>>> commit standard based on inspection alone? I'm okay either way, let >>>> me know. >>> >>> Doesn't seem that hard to test: run a dblink query that pulls back a >>> bunch of data under best-case conditions (ie, local not remote server), >>> and time it with and without the proposed fix. If the difference >>> is marginal then it's not worth working hard to optimize. >> >> Okay, will do, and here's the shorter and less mechanically intensive >> naive version that I think is the baseline: it doesn't try to optimize >> out any GUC settings and sets up the GUCs before the two >> materialization paths in dblink. > > The results. Summary: seems like grabbing the GUC and strcmp-ing is > worthwhile, but the amount of ping-ponging between processes adds some > noise to the timing results: utilization is far short of 100% on > either processor involved. Attached is a cumulative diff of the new > version, and also reproduced below are the changes to v2 that make up > v3.
I added programming around various NULL returns reading GUCs in this revision, v4. The non-cumulative changes: --- a/contrib/dblink/dblink.c +++ b/contrib/dblink/dblink.c @@ -3005,8 +3005,22 @@ applyRemoteGucs(remoteGucs *rgs) /* * Attempt to avoid GUC setting if the remote and local GUCs * already have the same value. + * + * NB: Must error if the GUC is not found. */ - localVal = GetConfigOption(gucName, true, true); + localVal = GetConfigOption(gucName, false, true); + + if (remoteVal == NULL) + ereport(ERROR, + (errmsg("could not load parameter status of %s", + gucName))); + + /* + * An error must have been raised by now if GUC values could + * not be loaded for any reason. + */ + Assert(localVal != NULL); + Assert(remoteVal != NULL); if (strcmp(remoteVal, localVal) == 0) continue; -- fdr
dblink-guc-sensitive-types-v4.patch
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