Re: [HACKERS] Extension upgrade and GUCs

2015-08-20 Thread Simon Riggs
On 18 August 2015 at 21:03, Paul Ramsey pram...@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 Riggshttp://www.2ndQuadrant.com/
http://www.2ndquadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training  Services


Re: [HACKERS] Extension upgrade and GUCs

2015-08-20 Thread Paul Ramsey
On August 20, 2015 at 2:17:31 AM, Simon Riggs 
(si...@2ndquadrant.com(mailto:si...@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


Re: [HACKERS] Extension upgrade and GUCs

2015-08-20 Thread Tom Lane
Paul Ramsey pram...@cleverelephant.ca writes:
 On August 20, 2015 at 2:17:31 AM, Simon Riggs 
 (si...@2ndquadrant.com(mailto:si...@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


Re: [HACKERS] Extension upgrade and GUCs

2015-08-20 Thread Paul Ramsey

On August 20, 2015 at 7:21:10 AM, Tom Lane 
(t...@sss.pgh.pa.us(mailto:t...@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


Re: [HACKERS] Extension upgrade and GUCs

2015-08-20 Thread Simon Riggs
On 20 August 2015 at 13:21, Paul Ramsey pram...@cleverelephant.ca wrote:

 On August 20, 2015 at 2:17:31 AM, Simon Riggs (si...@2ndquadrant.com
 (mailto:si...@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 Riggshttp://www.2ndQuadrant.com/
http://www.2ndquadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training  Services


[HACKERS] Extension upgrade and GUCs

2015-08-18 Thread Paul Ramsey
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