public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v3] Prevent jumbling of every element in ArrayExpr
3+ messages / 3 participants
[nested] [flat]
* [PATCH v3] Prevent jumbling of every element in ArrayExpr
@ 2020-11-17 15:18 Dmitrii Dolgov <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Dmitrii Dolgov @ 2020-11-17 15:18 UTC (permalink / raw)
pg_stat_statements produces multiple entries for queries like
SELECT something FROM table WHERE col IN (1, 2, 3, ...)
depending on number of parameters, because every element of ArrayExpr is
jumbled. Make Consts contribute nothing to the jumble hash if they're
part of a series and at position further that specified threshold. Do
the same for similar queries with VALUES as well.
---
.../expected/pg_stat_statements.out | 750 +++++++++++++++++-
.../pg_stat_statements/pg_stat_statements.c | 262 +++++-
.../sql/pg_stat_statements.sql | 139 ++++
3 files changed, 1137 insertions(+), 14 deletions(-)
diff --git a/contrib/pg_stat_statements/expected/pg_stat_statements.out b/contrib/pg_stat_statements/expected/pg_stat_statements.out
index 2a303a7f07..4b5ed40bb2 100644
--- a/contrib/pg_stat_statements/expected/pg_stat_statements.out
+++ b/contrib/pg_stat_statements/expected/pg_stat_statements.out
@@ -205,7 +205,7 @@ SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C";
INSERT INTO test VALUES(generate_series($1, $2), $3) | 1 | 10
SELECT * FROM test ORDER BY a | 1 | 12
SELECT * FROM test WHERE a > $1 ORDER BY a | 2 | 4
- SELECT * FROM test WHERE a IN ($1, $2, $3, $4, $5) | 1 | 8
+ SELECT * FROM test WHERE a IN ($1, $2, $3, $4, ...) | 1 | 8
SELECT pg_stat_statements_reset() | 1 | 1
SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C" | 0 | 0
UPDATE test SET b = $1 WHERE a = $2 | 6 | 6
@@ -861,4 +861,752 @@ SELECT query, plans, calls, rows FROM pg_stat_statements ORDER BY query COLLATE
SELECT query, plans, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C" | 1 | 0 | 0
(6 rows)
+--
+-- Consts merging
+--
+SET pg_stat_statements.merge_threshold = 5;
+CREATE TABLE test_merge (id int, data int);
+-- IN queries
+-- Normal
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3) | 1
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, ...) | 5
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(4 rows)
+
+-- On the merge threshold
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4) | 1
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, ...) | 6
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(4 rows)
+
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, ...) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, ...) | 6
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(3 rows)
+
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, ...) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, ...) | 5
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(3 rows)
+
+-- With gaps on the threshold
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4) | 1
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, ...) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(4 rows)
+
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, ...) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, ...) | 2
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(3 rows)
+
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, ...) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, ...) | 2
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(3 rows)
+
+-- test constants after merge
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10) and data = 2;
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+---------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, ...) and data = $11 | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+-- test prepared statement
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+PREPARE query AS
+SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10);
+EXECUTE query (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ PREPARE query AS +| 1
+ SELECT * FROM test_merge WHERE id IN ($6, $7, $8, ...) |
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+-- VALUES queries
+-- Normal
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM (VALUES (1), (2), (3)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+(3 rows)
+
+SELECT * FROM (VALUES (1), (2), (3), (4)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+(4 rows)
+
+SELECT * FROM (VALUES (1), (2), (3), (4), (5)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+ 5
+(5 rows)
+
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+ 5
+ 6
+(6 rows)
+
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+ 5
+ 6
+ 7
+(7 rows)
+
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7), (8)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+ 5
+ 6
+ 7
+ 8
+(8 rows)
+
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+ 5
+ 6
+ 7
+ 8
+ 9
+(9 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM (VALUES ($1), ($2), ($3)) q | 1
+ SELECT * FROM (VALUES ($1), ($2), ($3), ($4)) q | 1
+ SELECT * FROM (VALUES ($1), ($2), ($3), ($4), (...)) q | 5
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(5 rows)
+
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM (VALUES (1, 1), (2, 2), (3, 3)) q;
+ column1 | column2
+---------+---------
+ 1 | 1
+ 2 | 2
+ 3 | 3
+(3 rows)
+
+SELECT * FROM (VALUES (1, 1), (2, 2), (3, 3), (4, 4)) q;
+ column1 | column2
+---------+---------
+ 1 | 1
+ 2 | 2
+ 3 | 3
+ 4 | 4
+(4 rows)
+
+SELECT * FROM (VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5)) q;
+ column1 | column2
+---------+---------
+ 1 | 1
+ 2 | 2
+ 3 | 3
+ 4 | 4
+ 5 | 5
+(5 rows)
+
+SELECT * FROM (VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5), (6, 6)) q;
+ column1 | column2
+---------+---------
+ 1 | 1
+ 2 | 2
+ 3 | 3
+ 4 | 4
+ 5 | 5
+ 6 | 6
+(6 rows)
+
+SELECT * FROM (VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5), (6, 6), (7, 7)) q;
+ column1 | column2
+---------+---------
+ 1 | 1
+ 2 | 2
+ 3 | 3
+ 4 | 4
+ 5 | 5
+ 6 | 6
+ 7 | 7
+(7 rows)
+
+SELECT * FROM (VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5), (6, 6), (7, 7), (8, 8)) q;
+ column1 | column2
+---------+---------
+ 1 | 1
+ 2 | 2
+ 3 | 3
+ 4 | 4
+ 5 | 5
+ 6 | 6
+ 7 | 7
+ 8 | 8
+(8 rows)
+
+SELECT * FROM (VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5), (6, 6), (7, 7), (8, 8), (9, 9)) q;
+ column1 | column2
+---------+---------
+ 1 | 1
+ 2 | 2
+ 3 | 3
+ 4 | 4
+ 5 | 5
+ 6 | 6
+ 7 | 7
+ 8 | 8
+ 9 | 9
+(9 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM (VALUES ($1, $2), ($3, $4), ($5, $6)) q | 1
+ SELECT * FROM (VALUES ($1, $2), ($3, $4), ($5, $6), ($7, $8)) q | 1
+ SELECT * FROM (VALUES ($1, $2), ($3, $4), ($5, $6), ($7, $8), (...)) q | 5
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(5 rows)
+
+-- On the merge threshold
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM (VALUES (1), (2), (3), (4)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+(4 rows)
+
+SELECT * FROM (VALUES (1), (2), (3), (4), (5)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+ 5
+(5 rows)
+
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+ 5
+ 6
+(6 rows)
+
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+ 5
+ 6
+ 7
+(7 rows)
+
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7), (8)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+ 5
+ 6
+ 7
+ 8
+(8 rows)
+
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+ 5
+ 6
+ 7
+ 8
+ 9
+(9 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM (VALUES ($1), ($2), ($3), ($4)) q | 1
+ SELECT * FROM (VALUES ($1), ($2), ($3), ($4), (...)) q | 5
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(4 rows)
+
+-- With gaps on the treshold
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM (VALUES (1), (2), (3), (4)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+(4 rows)
+
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+ 5
+ 6
+ 7
+ 8
+ 9
+(9 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM (VALUES ($1), ($2), ($3), ($4)) q | 1
+ SELECT * FROM (VALUES ($1), ($2), ($3), ($4), (...)) q | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(4 rows)
+
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM (VALUES (1), (2), (3), (4), (5)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+ 5
+(5 rows)
+
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+ 5
+ 6
+ 7
+ 8
+ 9
+(9 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM (VALUES ($1), ($2), ($3), ($4), (...)) q | 2
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+ 5
+ 6
+(6 rows)
+
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9)) q;
+ column1
+---------
+ 1
+ 2
+ 3
+ 4
+ 5
+ 6
+ 7
+ 8
+ 9
+(9 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM (VALUES ($1), ($2), ($3), ($4), (...)) q | 2
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
DROP EXTENSION pg_stat_statements;
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index dd963c4644..103ecb7c61 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -231,6 +231,8 @@ typedef struct pgssLocationLen
{
int location; /* start offset in query text */
int length; /* length in bytes, or -1 to ignore */
+ bool merged; /* whether or not the location was marked as
+ duplicate */
} pgssLocationLen;
/*
@@ -245,7 +247,10 @@ typedef struct pgssJumbleState
/* Number of bytes used in jumble[] */
Size jumble_len;
- /* Array of locations of constants that should be removed */
+ /*
+ * Array of locations of constants that should be removed, or parameters
+ * that are already replaced, but could be also processed to be merged
+ */
pgssLocationLen *clocations;
/* Allocated length of clocations array */
@@ -299,6 +304,7 @@ static const struct config_enum_entry track_options[] =
static int pgss_max; /* max # statements to track */
static int pgss_track; /* tracking level */
+static int pgss_merge_threshold; /* minumum number of consts for merge */
static bool pgss_track_utility; /* whether to track utility commands */
static bool pgss_track_planning; /* whether to track planning duration */
static bool pgss_save; /* whether to save stats across shutdown */
@@ -374,7 +380,9 @@ static void JumbleQuery(pgssJumbleState *jstate, Query *query);
static void JumbleRangeTable(pgssJumbleState *jstate, List *rtable);
static void JumbleRowMarks(pgssJumbleState *jstate, List *rowMarks);
static void JumbleExpr(pgssJumbleState *jstate, Node *node);
-static void RecordConstLocation(pgssJumbleState *jstate, int location);
+static bool JumbleExprList(pgssJumbleState *jstate, Node *node);
+static void RecordConstLocation(pgssJumbleState *jstate, int location,
+ bool merged);
static char *generate_normalized_query(pgssJumbleState *jstate, const char *query,
int query_loc, int *query_len_p);
static void fill_in_constant_lengths(pgssJumbleState *jstate, const char *query,
@@ -460,6 +468,19 @@ _PG_init(void)
NULL,
NULL);
+ DefineCustomIntVariable("pg_stat_statements.merge_threshold",
+ "After this number of duplicate constants start to merge them.",
+ NULL,
+ &pgss_merge_threshold,
+ 5,
+ 1,
+ INT_MAX,
+ PGC_SUSET,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+
EmitWarningsOnPlaceholders("pg_stat_statements");
/*
@@ -863,7 +884,7 @@ pgss_post_parse_analyze(ParseState *pstate, Query *query)
jstate.jumble_len = 0;
jstate.clocations_buf_size = 32;
jstate.clocations = (pgssLocationLen *)
- palloc(jstate.clocations_buf_size * sizeof(pgssLocationLen));
+ palloc0(jstate.clocations_buf_size * sizeof(pgssLocationLen));
jstate.clocations_count = 0;
jstate.highest_extern_param_id = 0;
@@ -2655,7 +2676,7 @@ JumbleRangeTable(pgssJumbleState *jstate, List *rtable)
JumbleExpr(jstate, (Node *) rte->tablefunc);
break;
case RTE_VALUES:
- JumbleExpr(jstate, (Node *) rte->values_lists);
+ JumbleExprList(jstate, (Node *) rte->values_lists);
break;
case RTE_CTE:
@@ -2699,6 +2720,203 @@ JumbleRowMarks(pgssJumbleState *jstate, List *rowMarks)
}
}
+static bool
+JumbleExprList(pgssJumbleState *jstate, Node *node)
+{
+ ListCell *temp;
+ Node *firstExpr = NULL;
+ bool merged = false;
+ bool allConst = true;
+ int currentExprIdx;
+ int lastExprLenght = 0;
+
+ if (node == NULL)
+ return merged;
+
+ Assert(IsA(node, List));
+ firstExpr = (Node *) lfirst(list_head((List *) node));
+
+ /* Guard against stack overflow due to overly complex expressions */
+ check_stack_depth();
+
+ /*
+ * We always emit the node's NodeTag, then any additional fields that are
+ * considered significant, and then we recurse to any child nodes.
+ */
+ APP_JUMB(node->type);
+
+ /*
+ * If the first expression is a constant or a list of constants, try to
+ * merge the following if they're constants as well. Otherwise do
+ * JumbleExpr as usual.
+ */
+ switch (nodeTag(firstExpr))
+ {
+ case T_List:
+ currentExprIdx = 0;
+
+ foreach(temp, (List *) node)
+ {
+ List *expr = (List *) lfirst(temp);
+ ListCell *lc;
+
+ foreach(lc, expr)
+ {
+ Node * subExpr = (Node *) lfirst(lc);
+
+ if (!IsA(subExpr, Const))
+ {
+ allConst = false;
+ break;
+ }
+ }
+
+ if (!equal(expr, firstExpr) && allConst &&
+ currentExprIdx >= pgss_merge_threshold - 1)
+ {
+ merged = true;
+
+ /*
+ * This hash is going to accumulate the following merged
+ * statements
+ */
+ if (currentExprIdx == pgss_merge_threshold - 1)
+ {
+ JumbleExpr(jstate, (Node *) expr);
+
+ /*
+ * An expr consisting of constants is already found,
+ * JumbleExpr must record it. Mark all the constants as
+ * merged, they will be the first merged but still
+ * present in the statement query.
+ */
+ Assert(jstate->clocations_count > lastExprLenght - 1);
+ for (int i = 1; i < lastExprLenght + 1; i++)
+ {
+ pgssLocationLen *loc;
+ loc = &jstate->clocations[jstate->clocations_count - i];
+ loc->merged = true;
+ }
+ currentExprIdx++;
+ }
+ else
+ foreach(lc, expr)
+ {
+ Const *c = (Const *) lfirst(lc);
+ RecordConstLocation(jstate, c->location, true);
+ }
+
+ continue;
+ }
+
+ JumbleExpr(jstate, (Node *) expr);
+ currentExprIdx++;
+ lastExprLenght = expr->length;
+ }
+ break;
+
+ case T_Const:
+ currentExprIdx = 0;
+
+ foreach(temp, (List *) node)
+ {
+ Node *expr = (Node *) lfirst(temp);
+
+ if (!equal(expr, firstExpr) && IsA(expr, Const) &&
+ currentExprIdx >= pgss_merge_threshold - 1)
+ {
+ merged = true;
+
+ /*
+ * This hash is going to accumulate the following merged
+ * statements
+ */
+ if (currentExprIdx == pgss_merge_threshold - 1)
+ {
+ JumbleExpr(jstate, expr);
+
+ /*
+ * A const expr is already found, so JumbleExpr must
+ * record it. Mark it as merged, it will be the first
+ * merged but still present in the statement query.
+ */
+ Assert(jstate->clocations_count > 0);
+ jstate->clocations[jstate->clocations_count - 1].merged = true;
+ currentExprIdx++;
+ }
+ else
+ {
+ Const *c = (Const *) expr;
+ RecordConstLocation(jstate, c->location, true);
+ }
+
+ continue;
+ }
+
+ JumbleExpr(jstate, expr);
+ currentExprIdx++;
+ }
+ break;
+
+ case T_Param:
+ currentExprIdx = 0;
+
+ foreach(temp, (List *) node)
+ {
+ Node *expr = (Node *) lfirst(temp);
+ Param *p = (Param *) expr;
+
+ if (!equal(expr, firstExpr) && IsA(expr, Param) &&
+ currentExprIdx >= pgss_merge_threshold - 1)
+ {
+
+ merged = true;
+
+ /*
+ * This hash is going to accumulate the following merged
+ * statements
+ */
+ if (currentExprIdx == pgss_merge_threshold - 1)
+ {
+ JumbleExpr(jstate, expr);
+
+ /*
+ * A const expr is already found, so JumbleExpr must
+ * record it. Mark it as merged, it will be the first
+ * merged but still present in the statement query.
+ */
+ Assert(jstate->clocations_count > 0);
+ jstate->clocations[jstate->clocations_count - 1].merged = true;
+ currentExprIdx++;
+ }
+ else
+ RecordConstLocation(jstate, p->location, true);
+
+ continue;
+ }
+
+ JumbleExpr(jstate, expr);
+
+ /*
+ * To allow merging of parameters as well in
+ * generate_normalized_query, remember it as a constant.
+ */
+ RecordConstLocation(jstate, p->location, false);
+ currentExprIdx++;
+ }
+ break;
+
+ default:
+ foreach(temp, (List *) node)
+ {
+ JumbleExpr(jstate, (Node *) lfirst(temp));
+ }
+ break;
+ }
+
+ return merged;
+}
+
/*
* Jumble an expression tree
*
@@ -2748,7 +2966,7 @@ JumbleExpr(pgssJumbleState *jstate, Node *node)
/* We jumble only the constant's type, not its value */
APP_JUMB(c->consttype);
/* Also, record its parse location for query normalization */
- RecordConstLocation(jstate, c->location);
+ RecordConstLocation(jstate, c->location, false);
}
break;
case T_Param:
@@ -2936,7 +3154,7 @@ JumbleExpr(pgssJumbleState *jstate, Node *node)
}
break;
case T_ArrayExpr:
- JumbleExpr(jstate, (Node *) ((ArrayExpr *) node)->elements);
+ JumbleExprList(jstate, (Node *) ((ArrayExpr *) node)->elements);
break;
case T_RowExpr:
JumbleExpr(jstate, (Node *) ((RowExpr *) node)->args);
@@ -3189,11 +3407,11 @@ JumbleExpr(pgssJumbleState *jstate, Node *node)
}
/*
- * Record location of constant within query string of query tree
+ * Record location of constant or a parameter within query string of query tree
* that is currently being walked.
*/
static void
-RecordConstLocation(pgssJumbleState *jstate, int location)
+RecordConstLocation(pgssJumbleState *jstate, int location, bool merged)
{
/* -1 indicates unknown or undefined location */
if (location >= 0)
@@ -3210,6 +3428,8 @@ RecordConstLocation(pgssJumbleState *jstate, int location)
jstate->clocations[jstate->clocations_count].location = location;
/* initialize lengths to -1 to simplify fill_in_constant_lengths */
jstate->clocations[jstate->clocations_count].length = -1;
+ jstate->clocations[jstate->clocations_count].merged = merged;
+
jstate->clocations_count++;
}
}
@@ -3246,6 +3466,7 @@ generate_normalized_query(pgssJumbleState *jstate, const char *query,
n_quer_loc = 0, /* Normalized query byte location */
last_off = 0, /* Offset from start for previous tok */
last_tok_len = 0; /* Length (in bytes) of that tok */
+ bool merge = false;
/*
* Get constants' lengths (core system only gives us locations). Note
@@ -3284,12 +3505,27 @@ generate_normalized_query(pgssJumbleState *jstate, const char *query,
len_to_wrt -= last_tok_len;
Assert(len_to_wrt >= 0);
- memcpy(norm_query + n_quer_loc, query + quer_loc, len_to_wrt);
- n_quer_loc += len_to_wrt;
- /* And insert a param symbol in place of the constant token */
- n_quer_loc += sprintf(norm_query + n_quer_loc, "$%d",
- i + 1 + jstate->highest_extern_param_id);
+ if (!jstate->clocations[i].merged)
+ {
+ memcpy(norm_query + n_quer_loc, query + quer_loc, len_to_wrt);
+ n_quer_loc += len_to_wrt;
+
+ /* And insert a param symbol in place of the constant token */
+ n_quer_loc += sprintf(norm_query + n_quer_loc, "$%d",
+ i + 1 + jstate->highest_extern_param_id);
+ if (merge)
+ merge = false;
+ }
+ else if (!merge)
+ {
+ memcpy(norm_query + n_quer_loc, query + quer_loc, len_to_wrt);
+ n_quer_loc += len_to_wrt;
+
+ /* Merge until a non merged constant appear */
+ merge = true;
+ n_quer_loc += sprintf(norm_query + n_quer_loc, "...");
+ }
quer_loc = off + tok_len;
last_off = off;
diff --git a/contrib/pg_stat_statements/sql/pg_stat_statements.sql b/contrib/pg_stat_statements/sql/pg_stat_statements.sql
index e9f5bb84e3..ff5c9db6c0 100644
--- a/contrib/pg_stat_statements/sql/pg_stat_statements.sql
+++ b/contrib/pg_stat_statements/sql/pg_stat_statements.sql
@@ -358,4 +358,143 @@ SELECT 42;
SELECT 42;
SELECT query, plans, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C";
+--
+-- Consts merging
+--
+SET pg_stat_statements.merge_threshold = 5;
+CREATE TABLE test_merge (id int, data int);
+
+-- IN queries
+
+-- Normal
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- On the merge threshold
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- With gaps on the threshold
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- test constants after merge
+SELECT pg_stat_statements_reset();
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10) and data = 2;
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- test prepared statement
+SELECT pg_stat_statements_reset();
+
+PREPARE query AS
+SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10);
+EXECUTE query (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- VALUES queries
+
+-- Normal
+SELECT pg_stat_statements_reset();
+SELECT * FROM (VALUES (1), (2), (3)) q;
+SELECT * FROM (VALUES (1), (2), (3), (4)) q;
+SELECT * FROM (VALUES (1), (2), (3), (4), (5)) q;
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6)) q;
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7)) q;
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7), (8)) q;
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9)) q;
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT pg_stat_statements_reset();
+SELECT * FROM (VALUES (1, 1), (2, 2), (3, 3)) q;
+SELECT * FROM (VALUES (1, 1), (2, 2), (3, 3), (4, 4)) q;
+SELECT * FROM (VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5)) q;
+SELECT * FROM (VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5), (6, 6)) q;
+SELECT * FROM (VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5), (6, 6), (7, 7)) q;
+SELECT * FROM (VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5), (6, 6), (7, 7), (8, 8)) q;
+SELECT * FROM (VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5), (6, 6), (7, 7), (8, 8), (9, 9)) q;
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- On the merge threshold
+SELECT pg_stat_statements_reset();
+SELECT * FROM (VALUES (1), (2), (3), (4)) q;
+SELECT * FROM (VALUES (1), (2), (3), (4), (5)) q;
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6)) q;
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7)) q;
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7), (8)) q;
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9)) q;
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- With gaps on the treshold
+SELECT pg_stat_statements_reset();
+SELECT * FROM (VALUES (1), (2), (3), (4)) q;
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9)) q;
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT pg_stat_statements_reset();
+SELECT * FROM (VALUES (1), (2), (3), (4), (5)) q;
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9)) q;
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT pg_stat_statements_reset();
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6)) q;
+SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9)) q;
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
DROP EXTENSION pg_stat_statements;
--
2.21.0
--oyjisgzd2vntjkkc--
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: DSA_ALLOC_NO_OOM doesn't work
@ 2024-02-13 14:23 Heikki Linnakangas <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Heikki Linnakangas @ 2024-02-13 14:23 UTC (permalink / raw)
To: pgsql-hackers
(moving to pgsql-hackers)
On 29/01/2024 14:06, Heikki Linnakangas wrote:
> If you call dsa_allocate_extended(DSA_ALLOC_NO_OOM), it will still
> ereport an error if you run out of space (originally reported at [0]).
>
> Attached patch adds code to test_dsa.c to demonstrate that:
>
> postgres=# select test_dsa_basic();
> ERROR: could not resize shared memory segment "/PostgreSQL.1312700148"
> to 1075843072 bytes: No space left on device
>
> [0] https://github.com/pgvector/pgvector/issues/434#issuecomment-1912744489
I wrote the attached patch to address this, in a fairly straightforward
or naive way. The problem was that even though dsa_allocate() had code
to check the return value of dsm_create(), and return NULL instead of
ereport(ERROR) if the DSA_ALLOC_NO_OOM was set, dsm_create() does not in
fact return NULL on OOM. To fix, I added a DSM_CREATE_NO_OOM option to
dsm_create(), and I also had to punch that through to dsm_impl_op().
This is a little unpolished, but if we want to backpatch something
narrow now, this would probably be the right approach.
However, I must say that the dsm_impl_op() interface is absolutely
insane. It's like someone looked at ioctl() and thought, "hey that's a
great idea!". It mixes all different operations like creating or
destroying a DSM segment together into one function call, and the return
value is just a boolean, even though the function could fail for many
different reasons, and the callers do in fact care about the reason. In
a more natural interface, the different operations would have very
different function signatures.
I think we must refactor that. It might be best to leave this
DSA_ALLOC_NO_OOM bug unfixed in backpatches, and fix it on top of the
refactorings on master only. Later, we can backpatch the refactorings
too if we're comfortable with it; extensions shouldn't be using the
dsm_impl_op() interface directly.
(I skimmed through the thread where the DSM code was added, but didn't
see any mention of why dsm_impl_op() signature is like that:
https://www.postgresql.org/message-id/CA%2BTgmoaDqDUgt%3D4Zs_QPOnBt%3DEstEaVNP%2B5t%2Bm%3DFPNWshiPR3...)
--
Heikki Linnakangas
Neon (https://neon.tech)
Attachments:
[text/x-patch] 0001-Add-test.patch (1.0K, ../../[email protected]/2-0001-Add-test.patch)
download | inline diff:
From b60f877ad67e70b60915b2b25d0dbae6972c3536 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Tue, 13 Feb 2024 14:22:10 +0200
Subject: [PATCH 1/2] Add test
Discussion: https://www.postgresql.org/message-id/[email protected]
---
src/test/modules/test_dsa/test_dsa.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/test/modules/test_dsa/test_dsa.c b/src/test/modules/test_dsa/test_dsa.c
index 844316dec2b..f37eb57e99d 100644
--- a/src/test/modules/test_dsa/test_dsa.c
+++ b/src/test/modules/test_dsa/test_dsa.c
@@ -51,6 +51,19 @@ test_dsa_basic(PG_FUNCTION_ARGS)
for (int i = 0; i < 100; i++)
{
dsa_free(a, p[i]);
+ p[i] = InvalidDsaPointer;
+ }
+
+ for (int i = 0; i < 100; i++)
+ p[i] = dsa_allocate_extended(a, 1024*1024*1024, DSA_ALLOC_NO_OOM | DSA_ALLOC_HUGE);
+
+ for (int i = 0; i < 100; i++)
+ {
+ if (p[i] != InvalidDsaPointer)
+ {
+ dsa_free(a, p[i]);
+ p[i] = InvalidDsaPointer;
+ }
}
dsa_detach(a);
--
2.39.2
[text/x-patch] 0002-Fix-DSA_ALLOC_NO_OOM.patch (19.2K, ../../[email protected]/3-0002-Fix-DSA_ALLOC_NO_OOM.patch)
download | inline diff:
From fb20d55111d726dca247b2111a175f30d412e35a Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Tue, 13 Feb 2024 15:52:48 +0200
Subject: [PATCH 2/2] Fix DSA_ALLOC_NO_OOM
---
src/backend/access/common/session.c | 2 +-
src/backend/access/transam/parallel.c | 2 +-
src/backend/storage/ipc/dsm.c | 43 +++++++++-----
src/backend/storage/ipc/dsm_impl.c | 84 ++++++++++++++++++++-------
src/backend/utils/mmgr/dsa.c | 2 +-
src/include/storage/dsm.h | 1 +
src/include/storage/dsm_impl.h | 4 +-
7 files changed, 97 insertions(+), 41 deletions(-)
diff --git a/src/backend/access/common/session.c b/src/backend/access/common/session.c
index 3f2256f4915..5d4fe6dbb7a 100644
--- a/src/backend/access/common/session.c
+++ b/src/backend/access/common/session.c
@@ -102,7 +102,7 @@ GetSessionDsmHandle(void)
/* Set up segment and TOC. */
size = shm_toc_estimate(&estimator);
- seg = dsm_create(size, DSM_CREATE_NULL_IF_MAXSEGMENTS);
+ seg = dsm_create(size, DSM_CREATE_NULL_IF_MAXSEGMENTS | DSM_CREATE_NO_OOM);
if (seg == NULL)
{
MemoryContextSwitchTo(old_context);
diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 849a03e4b65..df1e5e7145d 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -312,7 +312,7 @@ InitializeParallelDSM(ParallelContext *pcxt)
*/
segsize = shm_toc_estimate(&pcxt->estimator);
if (pcxt->nworkers > 0)
- pcxt->seg = dsm_create(segsize, DSM_CREATE_NULL_IF_MAXSEGMENTS);
+ pcxt->seg = dsm_create(segsize, DSM_CREATE_NULL_IF_MAXSEGMENTS | DSM_CREATE_NO_OOM);
if (pcxt->seg != NULL)
pcxt->toc = shm_toc_create(PARALLEL_MAGIC,
dsm_segment_address(pcxt->seg),
diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c
index 6b12108dd10..2d2596fbe2d 100644
--- a/src/backend/storage/ipc/dsm.c
+++ b/src/backend/storage/ipc/dsm.c
@@ -214,7 +214,7 @@ dsm_postmaster_startup(PGShmemHeader *shim)
continue;
if (dsm_impl_op(DSM_OP_CREATE, dsm_control_handle, segsize,
&dsm_control_impl_private, &dsm_control_address,
- &dsm_control_mapped_size, ERROR))
+ &dsm_control_mapped_size, ERROR, 0))
break;
}
dsm_control = dsm_control_address;
@@ -255,7 +255,7 @@ dsm_cleanup_using_control_segment(dsm_handle old_control_handle)
* out quietly.
*/
if (!dsm_impl_op(DSM_OP_ATTACH, old_control_handle, 0, &impl_private,
- &mapped_address, &mapped_size, DEBUG1))
+ &mapped_address, &mapped_size, DEBUG1, 0))
return;
/*
@@ -266,7 +266,7 @@ dsm_cleanup_using_control_segment(dsm_handle old_control_handle)
if (!dsm_control_segment_sane(old_control, mapped_size))
{
dsm_impl_op(DSM_OP_DETACH, old_control_handle, 0, &impl_private,
- &mapped_address, &mapped_size, LOG);
+ &mapped_address, &mapped_size, LOG, 0);
return;
}
@@ -296,7 +296,7 @@ dsm_cleanup_using_control_segment(dsm_handle old_control_handle)
/* Destroy the referenced segment. */
dsm_impl_op(DSM_OP_DESTROY, handle, 0, &junk_impl_private,
- &junk_mapped_address, &junk_mapped_size, LOG);
+ &junk_mapped_address, &junk_mapped_size, LOG, 0);
}
/* Destroy the old control segment, too. */
@@ -304,7 +304,7 @@ dsm_cleanup_using_control_segment(dsm_handle old_control_handle)
"cleaning up dynamic shared memory control segment with ID %u",
old_control_handle);
dsm_impl_op(DSM_OP_DESTROY, old_control_handle, 0, &impl_private,
- &mapped_address, &mapped_size, LOG);
+ &mapped_address, &mapped_size, LOG, 0);
}
/*
@@ -400,7 +400,7 @@ dsm_postmaster_shutdown(int code, Datum arg)
/* Destroy the segment. */
dsm_impl_op(DSM_OP_DESTROY, handle, 0, &junk_impl_private,
- &junk_mapped_address, &junk_mapped_size, LOG);
+ &junk_mapped_address, &junk_mapped_size, LOG, 0);
}
/* Remove the control segment itself. */
@@ -410,7 +410,7 @@ dsm_postmaster_shutdown(int code, Datum arg)
dsm_control_address = dsm_control;
dsm_impl_op(DSM_OP_DESTROY, dsm_control_handle, 0,
&dsm_control_impl_private, &dsm_control_address,
- &dsm_control_mapped_size, LOG);
+ &dsm_control_mapped_size, LOG, 0);
dsm_control = dsm_control_address;
shim->dsm_control = 0;
}
@@ -512,6 +512,8 @@ dsm_shmem_init(void)
* remains attached until explicitly detached or the session ends.
* Creating with a NULL CurrentResourceOwner is equivalent to creating
* with a non-NULL CurrentResourceOwner and then calling dsm_pin_mapping.
+ *
+ * XXX: explain flags
*/
dsm_segment *
dsm_create(Size size, int flags)
@@ -569,14 +571,25 @@ dsm_create(Size size, int flags)
LWLockRelease(DynamicSharedMemoryControlLock);
for (;;)
{
+ int impl_flags = ((flags & DSM_CREATE_NO_OOM) != 0) ? DSM_OP_CREATE_NO_OOM : 0;
+
Assert(seg->mapped_address == NULL && seg->mapped_size == 0);
/* Use even numbers only */
seg->handle = pg_prng_uint32(&pg_global_prng_state) << 1;
if (seg->handle == DSM_HANDLE_INVALID) /* Reserve sentinel */
continue;
+ errno = 0;
if (dsm_impl_op(DSM_OP_CREATE, seg->handle, size, &seg->impl_private,
- &seg->mapped_address, &seg->mapped_size, ERROR))
+ &seg->mapped_address, &seg->mapped_size, ERROR, impl_flags))
break;
+ if ((flags & DSM_CREATE_NO_OOM) != 0 && (errno == ENOMEM || errno == ENOSPC))
+ {
+ if (seg->resowner != NULL)
+ ResourceOwnerForgetDSM(seg->resowner, seg);
+ dlist_delete(&seg->node);
+ pfree(seg);
+ return NULL;
+ }
}
LWLockAcquire(DynamicSharedMemoryControlLock, LW_EXCLUSIVE);
}
@@ -614,13 +627,13 @@ dsm_create(Size size, int flags)
LWLockRelease(DynamicSharedMemoryControlLock);
if (!using_main_dsm_region)
dsm_impl_op(DSM_OP_DESTROY, seg->handle, 0, &seg->impl_private,
- &seg->mapped_address, &seg->mapped_size, WARNING);
+ &seg->mapped_address, &seg->mapped_size, WARNING, 0);
if (seg->resowner != NULL)
ResourceOwnerForgetDSM(seg->resowner, seg);
dlist_delete(&seg->node);
pfree(seg);
- if ((flags & DSM_CREATE_NULL_IF_MAXSEGMENTS) != 0)
+ if ((flags & (DSM_CREATE_NULL_IF_MAXSEGMENTS | DSM_CREATE_NO_OOM)) != 0)
return NULL;
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_RESOURCES),
@@ -744,7 +757,7 @@ dsm_attach(dsm_handle h)
/* Here's where we actually try to map the segment. */
if (!is_main_region_dsm_handle(seg->handle))
dsm_impl_op(DSM_OP_ATTACH, seg->handle, 0, &seg->impl_private,
- &seg->mapped_address, &seg->mapped_size, ERROR);
+ &seg->mapped_address, &seg->mapped_size, ERROR, 0);
return seg;
}
@@ -788,7 +801,7 @@ dsm_detach_all(void)
if (control_address != NULL)
dsm_impl_op(DSM_OP_DETACH, dsm_control_handle, 0,
&dsm_control_impl_private, &control_address,
- &dsm_control_mapped_size, ERROR);
+ &dsm_control_mapped_size, ERROR, 0);
}
/*
@@ -841,7 +854,7 @@ dsm_detach(dsm_segment *seg)
{
if (!is_main_region_dsm_handle(seg->handle))
dsm_impl_op(DSM_OP_DETACH, seg->handle, 0, &seg->impl_private,
- &seg->mapped_address, &seg->mapped_size, WARNING);
+ &seg->mapped_address, &seg->mapped_size, WARNING, 0);
seg->impl_private = NULL;
seg->mapped_address = NULL;
seg->mapped_size = 0;
@@ -883,7 +896,7 @@ dsm_detach(dsm_segment *seg)
*/
if (is_main_region_dsm_handle(seg->handle) ||
dsm_impl_op(DSM_OP_DESTROY, seg->handle, 0, &seg->impl_private,
- &seg->mapped_address, &seg->mapped_size, WARNING))
+ &seg->mapped_address, &seg->mapped_size, WARNING, 0))
{
LWLockAcquire(DynamicSharedMemoryControlLock, LW_EXCLUSIVE);
if (is_main_region_dsm_handle(seg->handle))
@@ -1055,7 +1068,7 @@ dsm_unpin_segment(dsm_handle handle)
*/
if (is_main_region_dsm_handle(handle) ||
dsm_impl_op(DSM_OP_DESTROY, handle, 0, &junk_impl_private,
- &junk_mapped_address, &junk_mapped_size, WARNING))
+ &junk_mapped_address, &junk_mapped_size, WARNING, 0))
{
LWLockAcquire(DynamicSharedMemoryControlLock, LW_EXCLUSIVE);
if (is_main_region_dsm_handle(handle))
diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c
index 03aa47a1049..693caf8a64b 100644
--- a/src/backend/storage/ipc/dsm_impl.c
+++ b/src/backend/storage/ipc/dsm_impl.c
@@ -72,23 +72,23 @@
#ifdef USE_DSM_POSIX
static bool dsm_impl_posix(dsm_op op, dsm_handle handle, Size request_size,
void **impl_private, void **mapped_address,
- Size *mapped_size, int elevel);
+ Size *mapped_size, int elevel, int flags);
static int dsm_impl_posix_resize(int fd, off_t size);
#endif
#ifdef USE_DSM_SYSV
static bool dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size,
void **impl_private, void **mapped_address,
- Size *mapped_size, int elevel);
+ Size *mapped_size, int elevel, int flags);
#endif
#ifdef USE_DSM_WINDOWS
static bool dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size,
void **impl_private, void **mapped_address,
- Size *mapped_size, int elevel);
+ Size *mapped_size, int elevel, int flags);
#endif
#ifdef USE_DSM_MMAP
static bool dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size,
void **impl_private, void **mapped_address,
- Size *mapped_size, int elevel);
+ Size *mapped_size, int elevel, int flags);
#endif
static int errcode_for_dynamic_shared_memory(void);
@@ -153,38 +153,45 @@ int min_dynamic_shared_memory;
* a message should first be logged at the specified elevel, except in the
* case where DSM_OP_CREATE experiences a name collision, which should
* silently return false.
+ *
+ * If op is DSM_OP_CREATE and the DSM_OP_CREATE_NO_OOM flag is set, returns
+ * false on a failure caused by running out of memory or disk space,
+ * regardless of elevel. That can be distinguished from a name collision by
+ * checking if 'errno' is ENOSPC or ENOMEM.
*-----
*/
bool
dsm_impl_op(dsm_op op, dsm_handle handle, Size request_size,
void **impl_private, void **mapped_address, Size *mapped_size,
- int elevel)
+ int elevel, int flags)
{
Assert(op == DSM_OP_CREATE || request_size == 0);
Assert((op != DSM_OP_CREATE && op != DSM_OP_ATTACH) ||
(*mapped_address == NULL && *mapped_size == 0));
+ Assert((op == DSM_OP_CREATE && (flags & ~DSM_OP_CREATE_NO_OOM) == 0) ||
+ flags == 0);
switch (dynamic_shared_memory_type)
{
#ifdef USE_DSM_POSIX
case DSM_IMPL_POSIX:
return dsm_impl_posix(op, handle, request_size, impl_private,
- mapped_address, mapped_size, elevel);
+ mapped_address, mapped_size, elevel, flags);
#endif
#ifdef USE_DSM_SYSV
case DSM_IMPL_SYSV:
return dsm_impl_sysv(op, handle, request_size, impl_private,
- mapped_address, mapped_size, elevel);
+ mapped_address, mapped_size, elevel, flags);
#endif
#ifdef USE_DSM_WINDOWS
case DSM_IMPL_WINDOWS:
return dsm_impl_windows(op, handle, request_size, impl_private,
- mapped_address, mapped_size, elevel);
+ mapped_address, mapped_size, elevel, flags);
#endif
#ifdef USE_DSM_MMAP
case DSM_IMPL_MMAP:
return dsm_impl_mmap(op, handle, request_size, impl_private,
- mapped_address, mapped_size, elevel);
+ mapped_address, mapped_size, elevel, flags);
#endif
default:
elog(ERROR, "unexpected dynamic shared memory type: %d",
@@ -211,10 +218,10 @@ dsm_impl_op(dsm_op op, dsm_handle handle, Size request_size,
static bool
dsm_impl_posix(dsm_op op, dsm_handle handle, Size request_size,
void **impl_private, void **mapped_address, Size *mapped_size,
- int elevel)
+ int elevel, int flags)
{
char name[64];
- int flags;
+ int open_flags;
int fd;
char *address;
@@ -255,10 +262,13 @@ dsm_impl_posix(dsm_op op, dsm_handle handle, Size request_size,
*/
ReserveExternalFD();
- flags = O_RDWR | (op == DSM_OP_CREATE ? O_CREAT | O_EXCL : 0);
- if ((fd = shm_open(name, flags, PG_FILE_MODE_OWNER)) == -1)
+ open_flags = O_RDWR | (op == DSM_OP_CREATE ? O_CREAT | O_EXCL : 0);
+ if ((fd = shm_open(name, open_flags, PG_FILE_MODE_OWNER)) == -1)
{
ReleaseExternalFD();
+ if (op == DSM_OP_CREATE && (flags & DSM_OP_CREATE_NO_OOM) != 0 &&
+ (errno == ENOMEM || errno == ENOSPC))
+ return false;
if (op == DSM_OP_ATTACH || errno != EEXIST)
ereport(elevel,
(errcode_for_dynamic_shared_memory(),
@@ -304,6 +314,9 @@ dsm_impl_posix(dsm_op op, dsm_handle handle, Size request_size,
shm_unlink(name);
errno = save_errno;
+ if (op == DSM_OP_CREATE && (flags & DSM_OP_CREATE_NO_OOM) != 0 &&
+ (errno == ENOMEM || errno == ENOSPC))
+ return false;
ereport(elevel,
(errcode_for_dynamic_shared_memory(),
errmsg("could not resize shared memory segment \"%s\" to %zu bytes: %m",
@@ -326,6 +339,9 @@ dsm_impl_posix(dsm_op op, dsm_handle handle, Size request_size,
shm_unlink(name);
errno = save_errno;
+ if (op == DSM_OP_CREATE && (flags & DSM_OP_CREATE_NO_OOM) != 0 &&
+ (errno == ENOMEM || errno == ENOSPC))
+ return false;
ereport(elevel,
(errcode_for_dynamic_shared_memory(),
errmsg("could not map shared memory segment \"%s\": %m",
@@ -422,7 +438,7 @@ dsm_impl_posix_resize(int fd, off_t size)
static bool
dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size,
void **impl_private, void **mapped_address, Size *mapped_size,
- int elevel)
+ int elevel, int flags)
{
key_t key;
int ident;
@@ -484,13 +500,14 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size,
}
else
{
- int flags = IPCProtection;
+ int ipc_flags = IPCProtection;
size_t segsize;
/*
* Allocate the memory BEFORE acquiring the resource, so that we don't
* leak the resource if memory allocation fails.
*/
+ // FIXME: NO_OOM
ident_cache = MemoryContextAlloc(TopMemoryContext, sizeof(int));
/*
@@ -502,16 +519,20 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size,
if (op == DSM_OP_CREATE)
{
- flags |= IPC_CREAT | IPC_EXCL;
+ ipc_flags |= IPC_CREAT | IPC_EXCL;
segsize = request_size;
}
- if ((ident = shmget(key, segsize, flags)) == -1)
+ if ((ident = shmget(key, segsize, ipc_flags)) == -1)
{
+ if (op == DSM_OP_CREATE && (flags & DSM_OP_CREATE_NO_OOM) != 0 &&
+ (errno == ENOMEM || errno == ENOSPC))
+ return false;
if (op == DSM_OP_ATTACH || errno != EEXIST)
{
int save_errno = errno;
+ // FIXME: do we leak 'ident_cache' otherwise?
pfree(ident_cache);
errno = save_errno;
ereport(elevel,
@@ -579,6 +600,9 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size,
shmctl(ident, IPC_RMID, NULL);
errno = save_errno;
+ if (op == DSM_OP_CREATE && (flags & DSM_OP_CREATE_NO_OOM) != 0 &&
+ (errno == ENOMEM || errno == ENOSPC))
+ return false;
ereport(elevel,
(errcode_for_dynamic_shared_memory(),
errmsg("could not map shared memory segment \"%s\": %m",
@@ -609,7 +633,7 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size,
static bool
dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size,
void **impl_private, void **mapped_address,
- Size *mapped_size, int elevel)
+ Size *mapped_size, int elevel, int flags)
{
char *address;
HANDLE hmap;
@@ -702,6 +726,10 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size,
if (!hmap)
{
_dosmaperr(errcode);
+
+ if ((flags & DSM_OP_CREATE_NO_OOM) != 0 &&
+ (errno == ENOMEM || errno == ENOSPC))
+ return false;
ereport(elevel,
(errcode_for_dynamic_shared_memory(),
errmsg("could not create shared memory segment \"%s\": %m",
@@ -738,6 +766,9 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size,
CloseHandle(hmap);
errno = save_errno;
+ if (op == DSM_OP_CREATE && (flags & DSM_OP_CREATE_NO_OOM) != 0 &&
+ (errno == ENOMEM || errno == ENOSPC))
+ return false;
ereport(elevel,
(errcode_for_dynamic_shared_memory(),
errmsg("could not map shared memory segment \"%s\": %m",
@@ -791,10 +822,10 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size,
static bool
dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size,
void **impl_private, void **mapped_address, Size *mapped_size,
- int elevel)
+ int elevel, int flags)
{
char name[64];
- int flags;
+ int open_flags;
int fd;
char *address;
@@ -827,9 +858,12 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size,
}
/* Create new segment or open an existing one for attach. */
- flags = O_RDWR | (op == DSM_OP_CREATE ? O_CREAT | O_EXCL : 0);
- if ((fd = OpenTransientFile(name, flags)) == -1)
+ open_flags = O_RDWR | (op == DSM_OP_CREATE ? O_CREAT | O_EXCL : 0);
+ if ((fd = OpenTransientFile(name, open_flags)) == -1)
{
+ if (op == DSM_OP_CREATE && (flags & DSM_OP_CREATE_NO_OOM) != 0 &&
+ (errno == ENOMEM || errno == ENOSPC))
+ return false;
if (op == DSM_OP_ATTACH || errno != EEXIST)
ereport(elevel,
(errcode_for_dynamic_shared_memory(),
@@ -906,6 +940,9 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size,
unlink(name);
errno = save_errno ? save_errno : ENOSPC;
+ if (op == DSM_OP_CREATE && (flags & DSM_OP_CREATE_NO_OOM) != 0 &&
+ (errno == ENOMEM || errno == ENOSPC))
+ return false;
ereport(elevel,
(errcode_for_dynamic_shared_memory(),
errmsg("could not resize shared memory segment \"%s\" to %zu bytes: %m",
@@ -928,6 +965,9 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size,
unlink(name);
errno = save_errno;
+ if (op == DSM_OP_CREATE && (flags & DSM_OP_CREATE_NO_OOM) != 0 &&
+ (errno == ENOMEM || errno == ENOSPC))
+ return false;
ereport(elevel,
(errcode_for_dynamic_shared_memory(),
errmsg("could not map shared memory segment \"%s\": %m",
diff --git a/src/backend/utils/mmgr/dsa.c b/src/backend/utils/mmgr/dsa.c
index a6b728ba9ff..29b6f975be7 100644
--- a/src/backend/utils/mmgr/dsa.c
+++ b/src/backend/utils/mmgr/dsa.c
@@ -2164,7 +2164,7 @@ make_new_segment(dsa_area *area, size_t requested_pages)
/* Create the segment. */
oldowner = CurrentResourceOwner;
CurrentResourceOwner = area->resowner;
- segment = dsm_create(total_size, 0);
+ segment = dsm_create(total_size, DSM_CREATE_NULL_IF_MAXSEGMENTS | DSM_CREATE_NO_OOM);
CurrentResourceOwner = oldowner;
if (segment == NULL)
return NULL;
diff --git a/src/include/storage/dsm.h b/src/include/storage/dsm.h
index 1a22b32df1a..7e2ec8fc560 100644
--- a/src/include/storage/dsm.h
+++ b/src/include/storage/dsm.h
@@ -18,6 +18,7 @@
typedef struct dsm_segment dsm_segment;
#define DSM_CREATE_NULL_IF_MAXSEGMENTS 0x0001
+#define DSM_CREATE_NO_OOM 0x0002
/* Startup and shutdown functions. */
struct PGShmemHeader; /* avoid including pg_shmem.h */
diff --git a/src/include/storage/dsm_impl.h b/src/include/storage/dsm_impl.h
index 882269603da..2097876f756 100644
--- a/src/include/storage/dsm_impl.h
+++ b/src/include/storage/dsm_impl.h
@@ -66,10 +66,12 @@ typedef enum
DSM_OP_DESTROY,
} dsm_op;
+#define DSM_OP_CREATE_NO_OOM 0x01
+
/* Create, attach to, detach from, resize, or destroy a segment. */
extern bool dsm_impl_op(dsm_op op, dsm_handle handle, Size request_size,
void **impl_private, void **mapped_address, Size *mapped_size,
- int elevel);
+ int elevel, int flags);
/* Implementation-dependent actions required to keep segment until shutdown. */
extern void dsm_impl_pin_segment(dsm_handle handle, void *impl_private,
--
2.39.2
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: DSA_ALLOC_NO_OOM doesn't work
@ 2024-02-14 07:23 Robert Haas <[email protected]>
parent: Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Robert Haas @ 2024-02-14 07:23 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: pgsql-hackers
On Tue, Feb 13, 2024 at 7:53 PM Heikki Linnakangas <[email protected]> wrote:
> However, I must say that the dsm_impl_op() interface is absolutely
> insane. It's like someone looked at ioctl() and thought, "hey that's a
> great idea!".
As the person who wrote that code, this made me laugh.
I agree it's not the prettiest interface, but I thought that was OK
considering that it should only ever have a very limited number of
callers. I believe I did it this way in the interest of code
compactness. Since there are four DSM implementations, I wanted the
implementation-specific code to be short and all in one place, and
jamming it all into one function served that purpose. Also, there's a
bunch of logic that is shared by multiple operations - detach and
destroy tend to be similar, and so do create and attach, and there are
even things that are shared across all operations, like the snprintf
at the top of dsm_impl_posix() or the slightly larger amount of
boilerplate at the top of dsm_impl_sysv().
I'm not particularly opposed to refactoring this to make it nicer, but
my judgement was that splitting it up into one function per operation
per implementation, say, would have involved a lot of duplication of
small bits of code that might then get out of sync with each other
over time. By doing it this way, the logic is a bit tangled -- or
maybe more than a bit -- but there's very little duplication because
each implementation gets jammed into the smallest possible box. I'm OK
with somebody deciding that I got the trade-offs wrong there, but I
will be interested to see the number of lines added vs. removed in any
future refactoring patch.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2024-02-14 07:23 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-17 15:18 [PATCH v3] Prevent jumbling of every element in ArrayExpr Dmitrii Dolgov <[email protected]>
2024-02-13 14:23 Re: DSA_ALLOC_NO_OOM doesn't work Heikki Linnakangas <[email protected]>
2024-02-14 07:23 ` Re: DSA_ALLOC_NO_OOM doesn't work Robert Haas <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox