Extension upgrade and GUCs
Hi hackers, I've been wrestling with this one for a while and gone
down a couple blind alleys, so time to ask the experts.
PostGIS has a couple things different from the extensions that live in contrib,
- it has some GUCs
- it has a versioned loadable library (postgis-2.1.so, postgis-2.2.so, etc)
We've found that, when we run the following SQL,
CREATE EXTENSION postgis VERSION '2.1.9';
ALTER EXTENSION postgis UPDATE TO '2.2.0';
The update fails, because of a collision in the GUC.
When the extension is CREATEd, postgis-2.1.so is loaded, _PG_init() is
called, and that in turn calls DefineCustomStringVariable() to create
our GUC.
When the ALTER is called, the first time a C function definition is
called, the new library, postgis-2.2.so is loaded, the _PG_init() of
*that* library is called, and DefineCustomStringVariable() is called,
but this time it runs into the GUC definition from the first library
load, and the EXTENSION update process stops as an ERROR is thrown.
My initial attempt at avoiding this problem involved looking at
GetConfigOption() before running DefineCustomStringVariable() to see
if the GUC was already defined. This did not work, as it's possible to
define a GUC before loading the library. So an otherwise innocent
sequence of commands like:
SET my_guc = 'foo'; -- no library loaded yet
SELECT my_library_function(); -- causes library load and _PG_init() to fire
Would now fail, as it hit my test for a pre-existing GUC.
Unfortunately, the GUC we are using is not one where we simply read a
value now and a again. It switches the backend geometry library that
various functions use, so performance is a big deal: instead of
reading the GUC value, the code expects that GUC changes will flip a
global variable, using the GUC "assign" callback.
So I need a way to either (a) notice when I already have a (old) copy
of the library loaded and avoid trying to setup the GUC in that case
or (b) set-up the GUC in a somewhat less brittle way than
DefineCustomStringVariable() allows, something that can overwrite
things instead of just erroring out.
The ugly code in question is here
https://github.com/postgis/postgis/blob/svn-trunk/postgis/lwgeom_backend_api.c#L105
Discussion is here
https://trac.osgeo.org/postgis/ticket/2382
Thanks,
P
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 18 August 2015 at 21:03, Paul Ramsey <pramsey@cleverelephant.ca> wrote:
So I need a way to either (a) notice when I already have a (old) copy
of the library loaded and avoid trying to setup the GUC in that case
or (b) set-up the GUC in a somewhat less brittle way than
DefineCustomStringVariable() allows, something that can overwrite
things instead of just erroring out.
Are you trying to preserve the in-memory state across upgrade as well? It
sounds unlikely we can support that directly in the general case.
Sounds like we need RedefineCustomStringVariable()
--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On August 20, 2015 at 2:17:31 AM, Simon Riggs (simon@2ndquadrant.com(mailto:simon@2ndquadrant.com)) wrote:
On 18 August 2015 at 21:03, Paul Ramsey wrote:
So I need a way to either (a) notice when I already have a (old) copy
of the library loaded and avoid trying to setup the GUC in that case
or (b) set-up the GUC in a somewhat less brittle way than
DefineCustomStringVariable() allows, something that can overwrite
things instead of just erroring out.Are you trying to preserve the in-memory state across upgrade as well? It sounds unlikely we can support that directly in the general case.
I’m not sure what you mean by this.
Sounds like we need RedefineCustomStringVariable()
Yes, if that had existed we would not have had any problems (as long as it delegated back to Define..() in the case where the variable hadn’t been created yet…, since one of the problems is knowing if/to-what-extent a custom variable has already been defined).
We do now have a fix, which involved copying about 100 lines of core code (find_option, guc_var_compare, guc_name_compare) up, that does a low level search to see if there is a config_generic for a particular variable name, and if so whether it’s a placeholder or not. The presence of a non-placeholding definition is used as a “uh oh, there’s already a library in here” warning which keeps us from re-defining the variable and causing trouble.
P.
> --
Simon Riggs http://www.2ndQuadrant.com/(http://www.2ndquadrant.com/)
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 20 August 2015 at 13:21, Paul Ramsey <pramsey@cleverelephant.ca> wrote:
On August 20, 2015 at 2:17:31 AM, Simon Riggs (simon@2ndquadrant.com
(mailto:simon@2ndquadrant.com)) wrote:On 18 August 2015 at 21:03, Paul Ramsey wrote:
So I need a way to either (a) notice when I already have a (old) copy
of the library loaded and avoid trying to setup the GUC in that case
or (b) set-up the GUC in a somewhat less brittle way than
DefineCustomStringVariable() allows, something that can overwrite
things instead of just erroring out.Are you trying to preserve the in-memory state across upgrade as well?
It sounds unlikely we can support that directly in the general case.
I’m not sure what you mean by this.
The value of the global variable can't be maintained across upgrade.
Sounds like we need RedefineCustomStringVariable()
Yes, if that had existed we would not have had any problems (as long as it
delegated back to Define..() in the case where the variable hadn’t been
created yet…, since one of the problems is knowing if/to-what-extent a
custom variable has already been defined).We do now have a fix, which involved copying about 100 lines of core code
(find_option, guc_var_compare, guc_name_compare) up, that does a low level
search to see if there is a config_generic for a particular variable name,
and if so whether it’s a placeholder or not. The presence of a
non-placeholding definition is used as a “uh oh, there’s already a library
in here” warning which keeps us from re-defining the variable and causing
trouble.
I'm sure we all agree PostGIS is an important use case. Core is the right
place to put such code.
Please submit a patch that does that - better than someone else trying to
get it right for you. Thanks
--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Paul Ramsey <pramsey@cleverelephant.ca> writes:
On August 20, 2015 at 2:17:31 AM, Simon Riggs (simon@2ndquadrant.com(mailto:simon@2ndquadrant.com)) wrote:
Sounds like we need RedefineCustomStringVariable()
Yes, if that had existed we would not have had any problems (as long as it delegated back to Define..() in the case where the variable hadn’t been created yet…, since one of the problems is knowing if/to-what-extent a custom variable has already been defined).
I'm not sure that the situation you describe can be expected to work
reliably; the problems are far wider than just GUC variables. If two
different .so's are exposing broadly the same set of C function names,
how can we be sure which one will get called by the dynamic linker?
Isn't it likely that we'd end up continuing to call some functions
out of the old .so, probably leading to disaster?
I don't necessarily object to providing some solution for GUCs, but
I think first we need to question whether that isn't just the tip of
a large iceberg.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On August 20, 2015 at 7:21:10 AM, Tom Lane (tgl@sss.pgh.pa.us(mailto:tgl@sss.pgh.pa.us)) wrote:
I'm not sure that the situation you describe can be expected to work
reliably; the problems are far wider than just GUC variables. If two
different .so's are exposing broadly the same set of C function names,
how can we be sure which one will get called by the dynamic linker?
Isn't it likely that we'd end up continuing to call some functions
out of the old .so, probably leading to disaster?
Well, when you put it that way it sounds pretty scary :)
For whatever it’s worth, we’ve had versioned .so’s for a very long time, and every time an upgrade happens there is a period during which a backend will have two versions loaded simultaneously. But we only noticed recently when we got the GUC collision (our first GUC was only introduced in PostGIS 2.1). Perhaps because after upgrading most people disconnect, and then the next time they connect they only get the new library henceforth. In some cases (start a fresh backend and then do the upgrade immediately) it’s even possible to do an upgrade without triggering the double-load situation.
I don't necessarily object to providing some solution for GUCs, but
I think first we need to question whether that isn't just the tip of
a large iceberg.
There’s no doubt that the GUC issue is just a symptom of a potentially awful larger disease, but so far it’s the only symptom we’ve observed. Maybe because only a small % of functionality ever changes in a given release, combined with the relatively short lifespan of the double-loaded condition, and the fact the problem goes away immediately on re-connect, any problems have always just been ignored.
P.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers