PG-2005: Do not keep unnecessary entries in the query stack
PGSM support nested query tracking, and it has to track the current SQL call stack for this feature to work. But it incorrectly tracked all previous queries executed within the current top level statement instead o only the currently active queries. This was easily visible for example with a FOR LOOP in a user function, but could be also reproduced in many other ways. There's also an issue that the related assertion, that compares the length of the list with the max_stack_depth is incorrect. The stack depth limits the size of the postgres stack for the C code, not the number of nested SQL statements. For now this commit leaves these assertions as-is, as while they are not technically correct, these at least provide some kind of check on the nesting depth. Maybe it would make sense to remove them in the future, but for now, it could be a useful sanity check for testing the actual fix - we shouldn't hit this assertion anymore with the changes in this commit. As for the actual fix, with the changes in this commit pgsm removes list entries after we finished working on them. At that point we persisted everything we needed already into the shared memory, and no longer need the entries in the process local list. This is also true for the duplicated query string, which if needed was already copied to the shared memory.pull/580/head
parent
aa24b90c66
commit
5f8b716ef6
|
|
@ -203,6 +203,7 @@ char *unpack_sql_state(int sql_state);
|
|||
|
||||
static pgsmEntry *pgsm_create_hash_entry(uint64 bucket_id, int64 queryid, PlanInfo *plan_info);
|
||||
static void pgsm_add_to_list(pgsmEntry *entry, char *query_text, int query_len);
|
||||
static void pgsm_delete_entry(uint64 queryid);
|
||||
static pgsmEntry *pgsm_get_entry_for_query(int64 queryid, PlanInfo *plan_info, const char *query_text, int query_len, bool create, CmdType cmd_type);
|
||||
static int64 get_pgsm_query_id_hash(const char *norm_query, int len);
|
||||
|
||||
|
|
@ -804,6 +805,8 @@ pgsm_ExecutorEnd(QueryDesc *queryDesc)
|
|||
else
|
||||
standard_ExecutorEnd(queryDesc);
|
||||
|
||||
pgsm_delete_entry(queryDesc->plannedstmt->queryId);
|
||||
|
||||
num_relations = 0;
|
||||
}
|
||||
|
||||
|
|
@ -1274,6 +1277,7 @@ pgsm_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
|
|||
PG_END_TRY();
|
||||
#endif
|
||||
}
|
||||
pgsm_delete_entry(pstmt->queryId);
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
@ -1688,6 +1692,43 @@ pgsm_add_to_list(pgsmEntry *entry, char *query_text, int query_len)
|
|||
MemoryContextSwitchTo(oldctx);
|
||||
}
|
||||
|
||||
static void
|
||||
pgsm_delete_entry(uint64 queryid)
|
||||
{
|
||||
pgsmEntry *entry = NULL;
|
||||
ListCell *lc = NULL;
|
||||
|
||||
if (lentries == NIL)
|
||||
return;
|
||||
|
||||
entry = (pgsmEntry *) llast(lentries);
|
||||
if (entry->key.queryid == queryid)
|
||||
{
|
||||
pfree(entry->query_text.query_pointer);
|
||||
entry->query_text.query_pointer = NULL;
|
||||
lentries = list_delete_last(lentries);
|
||||
return;
|
||||
}
|
||||
|
||||
/*
|
||||
* The rest of the code is just paranoia. In theory this list is a stack,
|
||||
* and we always want to remove the last item. Similarly, in the getter
|
||||
* method we are always looking for the last item.
|
||||
*/
|
||||
|
||||
foreach(lc, lentries)
|
||||
{
|
||||
entry = lfirst(lc);
|
||||
if (entry->key.queryid == queryid)
|
||||
{
|
||||
pfree(entry->query_text.query_pointer);
|
||||
entry->query_text.query_pointer = NULL;
|
||||
lentries = list_delete_cell(lentries, lc);
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
static pgsmEntry *
|
||||
pgsm_get_entry_for_query(int64 queryid, PlanInfo *plan_info, const char *query_text, int query_len, bool create, CmdType cmd_type)
|
||||
{
|
||||
|
|
|
|||
|
|
@ -42,6 +42,19 @@ SELECT query, top_query FROM pg_stat_monitor ORDER BY query COLLATE "C";
|
|||
SELECT pg_stat_monitor_reset() |
|
||||
(5 rows)
|
||||
|
||||
-- make sure that we handle nested queries correctly
|
||||
BEGIN;
|
||||
DO $$
|
||||
DECLARE
|
||||
i int;
|
||||
BEGIN
|
||||
-- default stack limit is 2000kB, 50000 is much larger than that
|
||||
FOR i IN 1..50000 LOOP
|
||||
EXECUTE format('SELECT %s', i);
|
||||
END LOOP;
|
||||
END;
|
||||
$$;
|
||||
COMMIT;
|
||||
SELECT pg_stat_monitor_reset();
|
||||
pg_stat_monitor_reset
|
||||
-----------------------
|
||||
|
|
|
|||
|
|
@ -42,6 +42,19 @@ SELECT query, top_query FROM pg_stat_monitor ORDER BY query COLLATE "C";
|
|||
SELECT pg_stat_monitor_reset() |
|
||||
(5 rows)
|
||||
|
||||
-- make sure that we handle nested queries correctly
|
||||
BEGIN;
|
||||
DO $$
|
||||
DECLARE
|
||||
i int;
|
||||
BEGIN
|
||||
-- default stack limit is 2000kB, 50000 is much larger than that
|
||||
FOR i IN 1..50000 LOOP
|
||||
EXECUTE format('SELECT %s', i);
|
||||
END LOOP;
|
||||
END;
|
||||
$$;
|
||||
COMMIT;
|
||||
SELECT pg_stat_monitor_reset();
|
||||
pg_stat_monitor_reset
|
||||
-----------------------
|
||||
|
|
|
|||
|
|
@ -16,5 +16,22 @@ $$ language plpgsql;
|
|||
|
||||
SELECT add2(1,2);
|
||||
SELECT query, top_query FROM pg_stat_monitor ORDER BY query COLLATE "C";
|
||||
|
||||
-- make sure that we handle nested queries correctly
|
||||
|
||||
BEGIN;
|
||||
DO $$
|
||||
DECLARE
|
||||
i int;
|
||||
BEGIN
|
||||
-- default stack limit is 2000kB, 50000 is much larger than that
|
||||
FOR i IN 1..50000 LOOP
|
||||
EXECUTE format('SELECT %s', i);
|
||||
END LOOP;
|
||||
END;
|
||||
$$;
|
||||
|
||||
COMMIT;
|
||||
|
||||
SELECT pg_stat_monitor_reset();
|
||||
DROP EXTENSION pg_stat_monitor;
|
||||
|
|
|
|||
Loading…
Reference in New Issue