Opened 7 years ago
Closed 7 years ago
#3946 closed defect (fixed)
PostgreSQL 11 head no longer compiles against PostGIS trunk
Reported by: | robe | Owned by: | robe |
---|---|---|---|
Priority: | blocker | Milestone: | PostGIS 2.4.4 |
Component: | postgis | Version: | master |
Keywords: | Cc: |
Description
As of December 13th, debbie has been unable to compile against her PostgreSQL 11 git head builds.
Error looks like this: https://debbie.postgis.net/job/PostGIS_Regress_PGDEV_Weekly/5211/consoleFull
libtool: link: ranlib .libs/liblwgeom.a libtool: link: ( cd ".libs" && rm -f "liblwgeom.la" && ln -s "../liblwgeom.la" "liblwgeom.la" ) make[1]: Leaving directory '/var/lib/jenkins/workspace/postgis/regress_pgdev/branches/2.4/liblwgeom' ---- Making all in libpgcommon make[1]: Entering directory '/var/lib/jenkins/workspace/postgis/regress_pgdev/branches/2.4/libpgcommon' gcc -I../liblwgeom -g -O2 -I/var/lib/jenkins/workspace/pg/rel/pg11.0w64/include/postgresql/server -fPIC -DPIC -Wall -Wmissing-prototypes -c -o gserialized_gist.o gserialized_gist.c gcc -I../liblwgeom -g -O2 -I/var/lib/jenkins/workspace/pg/rel/pg11.0w64/include/postgresql/server -fPIC -DPIC -Wall -Wmissing-prototypes -c -o lwgeom_transform.o lwgeom_transform.c lwgeom_transform.c:115:2: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types] PROJ4SRSCacheDelete, ^ lwgeom_transform.c:115:2: note: (near initialization for ‘PROJ4SRSCacheContextMethods.get_chunk_space’) lwgeom_transform.c:117:2: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types] PROJ4SRSCacheIsEmpty, ^ lwgeom_transform.c:117:2: note: (near initialization for ‘PROJ4SRSCacheContextMethods.stats’) lwgeom_transform.c:118:2: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types] PROJ4SRSCacheStats ^ lwgeom_transform.c:118:2: note: (near initialization for ‘PROJ4SRSCacheContextMethods.check’) lwgeom_transform.c:120:3: warning: excess elements in struct initializer ,PROJ4SRSCacheCheck ^ lwgeom_transform.c:120:3: note: (near initialization for ‘PROJ4SRSCacheContextMethods’) lwgeom_transform.c: In function ‘AddToPROJ4SRSCache’: lwgeom_transform.c:550:40: warning: passing argument 1 of ‘MemoryContextCreate’ makes pointer from integer without a cast [-Wint-conversion] PJMemoryContext = MemoryContextCreate(T_AllocSetContext, 8192, ^ In file included from lwgeom_transform.c:18:0: /var/lib/jenkins/workspace/pg/rel/pg11.0w64/include/postgresql/server/utils/memutils.h:135:13: note: expected ‘MemoryContext {aka struct MemoryContextData *}’ but argument is of type ‘int’ extern void MemoryContextCreate(MemoryContext node, ^ lwgeom_transform.c:551:40: warning: passing argument 3 of ‘MemoryContextCreate’ makes integer from pointer without a cast [-Wint-conversion] &PROJ4SRSCacheContextMethods, ^ In file included from lwgeom_transform.c:18:0: /var/lib/jenkins/workspace/pg/rel/pg11.0w64/include/postgresql/server/utils/memutils.h:135:13: note: expected ‘Size {aka long unsigned int}’ but argument is of type ‘MemoryContextMethods * {aka struct MemoryContextMethods *}’ extern void MemoryContextCreate(MemoryContext node, ^ lwgeom_transform.c:552:40: warning: passing argument 4 of ‘MemoryContextCreate’ makes integer from pointer without a cast [-Wint-conversion] PROJ4Cache->PROJ4SRSCacheContext, ^ In file included from lwgeom_transform.c:18:0: /var/lib/jenkins/workspace/pg/rel/pg11.0w64/include/postgresql/server/utils/memutils.h:135:13: note: expected ‘Size {aka long unsigned int}’ but argument is of type ‘MemoryContext {aka struct MemoryContextData *}’ extern void MemoryContextCreate(MemoryContext node, ^ lwgeom_transform.c:553:40: warning: passing argument 5 of ‘MemoryContextCreate’ from incompatible pointer type [-Wincompatible-pointer-types] "PostGIS PROJ4 PJ Memory Context"); ^ In file included from lwgeom_transform.c:18:0: /var/lib/jenkins/workspace/pg/rel/pg11.0w64/include/postgresql/server/utils/memutils.h:135:13: note: expected ‘const MemoryContextMethods * {aka const struct MemoryContextMethods *}’ but argument is of type ‘char *’ extern void MemoryContextCreate(MemoryContext node, ^ lwgeom_transform.c:550:20: error: too few arguments to function ‘MemoryContextCreate’ PJMemoryContext = MemoryContextCreate(T_AllocSetContext, 8192, ^ In file included from lwgeom_transform.c:18:0: /var/lib/jenkins/workspace/pg/rel/pg11.0w64/include/postgresql/server/utils/memutils.h:135:13: note: declared here extern void MemoryContextCreate(MemoryContext node, ^ Makefile:61: recipe for target 'lwgeom_transform.o' failed make[1]: *** [lwgeom_transform.o] Error 1 make[1]: Leaving directory '/var/lib/jenkins/workspace/postgis/regress_pgdev/branches/2.4/libpgcommon' GNUmakefile:16: recipe for target 'all' failed make: *** [all] Error 1 Build step 'Execute shell' marked build as failure IRC notifier plugin: Sending notification to: #postgis-activity Finished: FAILURE
Which started as a result of this PostgreSQL commit
Changes
1) Rethink MemoryContext creation to improve performance. https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=9fa6f00b1308dd10da4eca2f31ccbfc7b35bb461
Change History (12)
comment:1 by , 7 years ago
Owner: | changed from | to
---|
comment:2 by , 7 years ago
comment:4 by , 7 years ago
I only see MemoryContextCreate
in three places, but in at least two of them (geos prepared and proj) they are being used in ways that are not consistent with AllocSetContextCreate
. They are being used as memory management tricks by the caching code for geos prepared geometry and proj handles. The problem is that geos and proj both allocate outside the pgsql memory manager, and we want to free up any resources in the cache once a pgsql statement ends. But we have no way of knowing when a statement is complete. So we create our own memory context and tuck it into the statement context. Now, when the statement ends, it has to first tell our own context to clean itself up. And into the cleanup routine of our context we add the code that frees up our cached geos/proj objects.
Basically, we don't use those contexts for storing things, we just use them to hide a callback. So we cannot change those over to AllocSetContextCreate
calls, we have to figure out how to continue to use them, or figure some other way to get a callback when a statement ends.
comment:5 by , 7 years ago
Two options I can see:
- Keep using memory context callbacks, but do so by creating a
AllocSetContext
in the current transaction context, and then reaching into it, over-riding it's delete callback and replacing it with our own, that first does the memory/cache cleaning we want and then calls the original delete function - Figuring out if we can use a system hook, like
ExecutorFinish_hook
orExecutorEnd_hook
to callback. This might not work as well, since we keep the cache for a long time and use the memory context trick to know which cache entry was associated with which callback. Might need a new trick for that too.
comment:6 by , 7 years ago
Third option, from convo in hackers:
- Using MemoryContextRegisterResetCallback on a AllocSetContext created under our PortalContext should do the trick, with less direct mucking about into the internals of contexts.
Only good for 9.5+, and might need some massaging to still work with old cache scheme.
comment:11 by , 7 years ago
Milestone: | PostGIS 2.5.0 → PostGIS 2.4.3 |
---|
I'm moving the milestone back as this will be required in 2.4 to support v11 in the future.
comment:12 by , 7 years ago
Milestone: | PostGIS 2.4.3 → PostGIS 2.4.4 |
---|
Asked my clueless question here
https://www.postgresql.org/message-id/000001d378a2%24db3d5060%2491b7f120%24%40pcorp.us
that perhaps we shouldn't be directly using MemoryContextCreate and instead be using AllocSetContextCreate or AllocSetContextCreateExtended