Skip to content

Commit aa4213a

Browse files
funbringerfunbringer
committed
fix memory leak caused by invalidations, fix parents cache
1 parent d03fd32 commit aa4213a

File tree

5 files changed

+190
-105
lines changed

5 files changed

+190
-105
lines changed

‎expected/pathman_calamity.out‎

Lines changed: 124 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -851,9 +851,7 @@ DROP EXTENSION pg_pathman;
851851
*/
852852
CREATE SCHEMA calamity;
853853
CREATE EXTENSION pg_pathman;
854-
/* Change this setting for code coverage */
855-
SET pg_pathman.enable_bounds_cache = false;
856-
/* check view pathman_cache_stats */
854+
/* check that cache loading is lazy */
857855
CREATE TABLE calamity.test_pathman_cache_stats(val NUMERIC NOT NULL);
858856
SELECT create_range_partitions('calamity.test_pathman_cache_stats', 'val', 1, 10, 10);
859857
create_range_partitions
@@ -870,22 +868,54 @@ SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
870868
partition parents cache | 0
871869
(4 rows)
872870

873-
SELECT drop_partitions('calamity.test_pathman_cache_stats');
874-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_1
875-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_2
876-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_3
877-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_4
878-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_5
879-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_6
880-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_7
881-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_8
882-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_9
883-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_10
884-
drop_partitions
885-
-----------------
886-
10
871+
DROP TABLE calamity.test_pathman_cache_stats CASCADE;
872+
NOTICE: drop cascades to 11 other objects
873+
SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
874+
context | entries
875+
--------------------------+---------
876+
maintenance | 0
877+
partition bounds cache | 0
878+
partition dispatch cache | 0
879+
partition parents cache | 0
880+
(4 rows)
881+
882+
/* Change this setting for code coverage */
883+
SET pg_pathman.enable_bounds_cache = false;
884+
/* check view pathman_cache_stats (bounds cache disabled) */
885+
CREATE TABLE calamity.test_pathman_cache_stats(val NUMERIC NOT NULL);
886+
SELECT create_range_partitions('calamity.test_pathman_cache_stats', 'val', 1, 10, 10);
887+
create_range_partitions
888+
-------------------------
889+
10
887890
(1 row)
888891

892+
EXPLAIN (COSTS OFF) SELECT * FROM calamity.test_pathman_cache_stats;
893+
QUERY PLAN
894+
-----------------------------------------------
895+
Append
896+
-> Seq Scan on test_pathman_cache_stats_1
897+
-> Seq Scan on test_pathman_cache_stats_2
898+
-> Seq Scan on test_pathman_cache_stats_3
899+
-> Seq Scan on test_pathman_cache_stats_4
900+
-> Seq Scan on test_pathman_cache_stats_5
901+
-> Seq Scan on test_pathman_cache_stats_6
902+
-> Seq Scan on test_pathman_cache_stats_7
903+
-> Seq Scan on test_pathman_cache_stats_8
904+
-> Seq Scan on test_pathman_cache_stats_9
905+
-> Seq Scan on test_pathman_cache_stats_10
906+
(11 rows)
907+
908+
SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
909+
context | entries
910+
--------------------------+---------
911+
maintenance | 0
912+
partition bounds cache | 0
913+
partition dispatch cache | 1
914+
partition parents cache | 10
915+
(4 rows)
916+
917+
DROP TABLE calamity.test_pathman_cache_stats CASCADE;
918+
NOTICE: drop cascades to 11 other objects
889919
SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
890920
context | entries
891921
--------------------------+---------
@@ -895,42 +925,102 @@ SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
895925
partition parents cache | 0
896926
(4 rows)
897927

898-
DROP TABLE calamity.test_pathman_cache_stats;
899928
/* Restore this GUC */
900929
SET pg_pathman.enable_bounds_cache = true;
901-
/* check view pathman_cache_stats (one more time) */
930+
/* check view pathman_cache_stats (bounds cache enabled) */
902931
CREATE TABLE calamity.test_pathman_cache_stats(val NUMERIC NOT NULL);
903932
SELECT create_range_partitions('calamity.test_pathman_cache_stats', 'val', 1, 10, 10);
904933
create_range_partitions
905934
-------------------------
906935
10
907936
(1 row)
908937

938+
EXPLAIN (COSTS OFF) SELECT * FROM calamity.test_pathman_cache_stats;
939+
QUERY PLAN
940+
-----------------------------------------------
941+
Append
942+
-> Seq Scan on test_pathman_cache_stats_1
943+
-> Seq Scan on test_pathman_cache_stats_2
944+
-> Seq Scan on test_pathman_cache_stats_3
945+
-> Seq Scan on test_pathman_cache_stats_4
946+
-> Seq Scan on test_pathman_cache_stats_5
947+
-> Seq Scan on test_pathman_cache_stats_6
948+
-> Seq Scan on test_pathman_cache_stats_7
949+
-> Seq Scan on test_pathman_cache_stats_8
950+
-> Seq Scan on test_pathman_cache_stats_9
951+
-> Seq Scan on test_pathman_cache_stats_10
952+
(11 rows)
953+
909954
SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
910955
context | entries
911956
--------------------------+---------
912957
maintenance | 0
913-
partition bounds cache | 0
958+
partition bounds cache | 10
914959
partition dispatch cache | 1
960+
partition parents cache | 10
961+
(4 rows)
962+
963+
DROP TABLE calamity.test_pathman_cache_stats CASCADE;
964+
NOTICE: drop cascades to 11 other objects
965+
SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
966+
context | entries
967+
--------------------------+---------
968+
maintenance | 0
969+
partition bounds cache | 0
970+
partition dispatch cache | 0
915971
partition parents cache | 0
916972
(4 rows)
917973

918-
SELECT drop_partitions('calamity.test_pathman_cache_stats');
919-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_1
920-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_2
921-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_3
922-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_4
923-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_5
924-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_6
925-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_7
926-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_8
927-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_9
928-
NOTICE: 0 rows copied from calamity.test_pathman_cache_stats_10
929-
drop_partitions
930-
-----------------
931-
10
974+
/* check that parents cache has been flushed after partition was dropped */
975+
CREATE TABLE calamity.test_pathman_cache_stats(val NUMERIC NOT NULL);
976+
SELECT create_range_partitions('calamity.test_pathman_cache_stats', 'val', 1, 10, 10);
977+
create_range_partitions
978+
-------------------------
979+
10
932980
(1 row)
933981

982+
EXPLAIN (COSTS OFF) SELECT * FROM calamity.test_pathman_cache_stats;
983+
QUERY PLAN
984+
-----------------------------------------------
985+
Append
986+
-> Seq Scan on test_pathman_cache_stats_1
987+
-> Seq Scan on test_pathman_cache_stats_2
988+
-> Seq Scan on test_pathman_cache_stats_3
989+
-> Seq Scan on test_pathman_cache_stats_4
990+
-> Seq Scan on test_pathman_cache_stats_5
991+
-> Seq Scan on test_pathman_cache_stats_6
992+
-> Seq Scan on test_pathman_cache_stats_7
993+
-> Seq Scan on test_pathman_cache_stats_8
994+
-> Seq Scan on test_pathman_cache_stats_9
995+
-> Seq Scan on test_pathman_cache_stats_10
996+
(11 rows)
997+
998+
SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
999+
context | entries
1000+
--------------------------+---------
1001+
maintenance | 0
1002+
partition bounds cache | 10
1003+
partition dispatch cache | 1
1004+
partition parents cache | 10
1005+
(4 rows)
1006+
1007+
SELECT drop_range_partition('calamity.test_pathman_cache_stats_1');
1008+
drop_range_partition
1009+
-------------------------------------
1010+
calamity.test_pathman_cache_stats_1
1011+
(1 row)
1012+
1013+
SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
1014+
context | entries
1015+
--------------------------+---------
1016+
maintenance | 0
1017+
partition bounds cache | 9
1018+
partition dispatch cache | 1
1019+
partition parents cache | 0
1020+
(4 rows)
1021+
1022+
DROP TABLE calamity.test_pathman_cache_stats CASCADE;
1023+
NOTICE: drop cascades to 10 other objects
9341024
SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
9351025
context | entries
9361026
--------------------------+---------
@@ -940,7 +1030,6 @@ SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
9401030
partition parents cache | 0
9411031
(4 rows)
9421032

943-
DROP TABLE calamity.test_pathman_cache_stats;
9441033
DROP SCHEMA calamity CASCADE;
9451034
DROP EXTENSION pg_pathman;
9461035
/*

‎sql/pathman_calamity.sql‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -372,27 +372,44 @@ DROP EXTENSION pg_pathman;
372372
CREATE SCHEMA calamity;
373373
CREATE EXTENSION pg_pathman;
374374

375+
/* check that cache loading is lazy */
376+
CREATE TABLE calamity.test_pathman_cache_stats(val NUMERIC NOT NULL);
377+
SELECT create_range_partitions('calamity.test_pathman_cache_stats', 'val', 1, 10, 10);
378+
SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
379+
DROP TABLE calamity.test_pathman_cache_stats CASCADE;
380+
SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
381+
375382
/* Change this setting for code coverage */
376383
SET pg_pathman.enable_bounds_cache = false;
377384

378-
/* check view pathman_cache_stats */
385+
/* check view pathman_cache_stats (bounds cache disabled) */
379386
CREATE TABLE calamity.test_pathman_cache_stats(val NUMERIC NOT NULL);
380387
SELECT create_range_partitions('calamity.test_pathman_cache_stats', 'val', 1, 10, 10);
388+
EXPLAIN (COSTS OFF) SELECT * FROM calamity.test_pathman_cache_stats;
381389
SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
382-
SELECT drop_partitions('calamity.test_pathman_cache_stats');
390+
DROP TABLE calamity.test_pathman_cache_stats CASCADE;
383391
SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
384-
DROP TABLE calamity.test_pathman_cache_stats;
385392

386393
/* Restore this GUC */
387394
SET pg_pathman.enable_bounds_cache = true;
388395

389-
/* check view pathman_cache_stats (one more time) */
396+
/* check view pathman_cache_stats (bounds cache enabled) */
390397
CREATE TABLE calamity.test_pathman_cache_stats(val NUMERIC NOT NULL);
391398
SELECT create_range_partitions('calamity.test_pathman_cache_stats', 'val', 1, 10, 10);
399+
EXPLAIN (COSTS OFF) SELECT * FROM calamity.test_pathman_cache_stats;
400+
SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
401+
DROP TABLE calamity.test_pathman_cache_stats CASCADE;
402+
SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
403+
404+
/* check that parents cache has been flushed after partition was dropped */
405+
CREATE TABLE calamity.test_pathman_cache_stats(val NUMERIC NOT NULL);
406+
SELECT create_range_partitions('calamity.test_pathman_cache_stats', 'val', 1, 10, 10);
407+
EXPLAIN (COSTS OFF) SELECT * FROM calamity.test_pathman_cache_stats;
408+
SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
409+
SELECT drop_range_partition('calamity.test_pathman_cache_stats_1');
392410
SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
393-
SELECT drop_partitions('calamity.test_pathman_cache_stats');
411+
DROP TABLE calamity.test_pathman_cache_stats CASCADE;
394412
SELECT context, entries FROM pathman_cache_stats ORDER BY context; /* OK */
395-
DROP TABLE calamity.test_pathman_cache_stats;
396413

397414
DROP SCHEMA calamity CASCADE;
398415
DROP EXTENSION pg_pathman;

‎src/include/compat/pg_compat.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@
2525
#if PG_VERSION_NUM < 90600
2626
#define ALLOCSET_DEFAULT_SIZES \
2727
ALLOCSET_DEFAULT_MINSIZE, ALLOCSET_DEFAULT_INITSIZE, ALLOCSET_DEFAULT_MAXSIZE
28+
29+
#define ALLOCSET_SMALL_SIZES \
30+
ALLOCSET_SMALL_MINSIZE, ALLOCSET_SMALL_INITSIZE, ALLOCSET_SMALL_MAXSIZE
2831
#endif
2932

3033

‎src/include/relation_info.h‎

Lines changed: 8 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "fmgr.h"
1919
#include "nodes/bitmapset.h"
2020
#include "nodes/nodes.h"
21+
#include "nodes/memnodes.h"
2122
#include "nodes/primnodes.h"
2223
#include "nodes/value.h"
2324
#include "port/atomics.h"
@@ -162,6 +163,8 @@ typedef struct
162163

163164
Oid cmp_proc, /* comparison fuction for 'ev_type' */
164165
hash_proc; /* hash function for 'ev_type' */
166+
167+
MemoryContext mcxt; /* memory context holding this struct */
165168
} PartRelationInfo;
166169

167170
#define PART_EXPR_VARNO ( 1 )
@@ -361,19 +364,13 @@ void shout_if_prel_is_invalid(const Oid parent_oid,
361364
* Useful functions & macros for freeing memory.
362365
*/
363366

364-
#define FreeIfNotNull(ptr) \
365-
do { \
366-
if (ptr) \
367-
{ \
368-
pfree((void *) ptr); \
369-
ptr = NULL; \
370-
} \
371-
} while(0)
372-
367+
/* Remove all references to this parent from parents cache */
373368
static inline void
374-
FreeChildrenArray(PartRelationInfo *prel)
369+
ForgetParent(PartRelationInfo *prel)
375370
{
376-
uint32 i;
371+
uint32 i;
372+
373+
AssertArg(MemoryContextIsValid(prel->mcxt));
377374

378375
/* Remove relevant PartParentInfos */
379376
if (prel->children)
@@ -390,38 +387,6 @@ FreeChildrenArray(PartRelationInfo *prel)
390387
if (PrelParentRelid(prel) == get_parent_of_partition(child, NULL))
391388
forget_parent_of_partition(child, NULL);
392389
}
393-
394-
pfree(prel->children);
395-
prel->children = NULL;
396-
}
397-
}
398-
399-
static inline void
400-
FreeRangesArray(PartRelationInfo *prel)
401-
{
402-
uint32 i;
403-
404-
/* Remove RangeEntries array */
405-
if (prel->ranges)
406-
{
407-
/* Remove persistent entries if not byVal */
408-
if (!prel->ev_byval)
409-
{
410-
for (i = 0; i < PrelChildrenCount(prel); i++)
411-
{
412-
Oid child = prel->ranges[i].child_oid;
413-
414-
/* Skip if Oid is invalid (e.g. initialization error) */
415-
if (!OidIsValid(child))
416-
continue;
417-
418-
FreeBound(&prel->ranges[i].min, prel->ev_byval);
419-
FreeBound(&prel->ranges[i].max, prel->ev_byval);
420-
}
421-
}
422-
423-
pfree(prel->ranges);
424-
prel->ranges = NULL;
425390
}
426391
}
427392

0 commit comments

Comments
 (0)