On Tue, Mar 19, 2013 at 6:10 PM, Daniel Farina <[email protected]> wrote: > On Tue, Mar 19, 2013 at 3:06 PM, Daniel Farina <[email protected]> wrote: >> On Tue, Mar 19, 2013 at 2:41 PM, Tom Lane <[email protected]> wrote: >>> Daniel Farina <[email protected]> writes: >>>> On Tue, Mar 19, 2013 at 1:16 PM, Tom Lane <[email protected]> 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 ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
