Обсуждение: 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 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

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.