Thread: 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
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/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
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 directlyin 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 wherethe variable hadn’t been created yet…, since one of the problems is knowing if/to-what-extent a custom variable hasalready 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, andif so whether it’s a placeholder or not. The presence of a non-placeholding definition is used as a “uh oh, there’s alreadya 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
On 20 August 2015 at 13:21, Paul Ramsey <pramsey@cleverelephant.ca> wrote:
Please submit a patch that does that - better than someone else trying to get it right for you. Thanks
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.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
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 wherethe variable hadn’t been created yet…, since one of the problems is knowing if/to-what-extent a custom variable hasalready 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
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 periodduring which a backend will have two versions loaded simultaneously. But we only noticed recently when we got the GUCcollision (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 backendand 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 symptomwe’ve observed. Maybe because only a small % of functionality ever changes in a given release, combined with the relativelyshort lifespan of the double-loaded condition, and the fact the problem goes away immediately on re-connect, anyproblems have always just been ignored. P.