Skip to content

Commit

Permalink
Fix leaks with functions in DML
Browse files Browse the repository at this point in the history
If plpgsql functions are used in DML queries then we were leaking 8KB
for every invocation of that function. This can quickly add up.

The issue was that the "CurTransactionContext" was not getting cleaned
up after every invocation. The reason was that we were inadvertantly
allocating a temporary list in that context. Postgres then thought that
this CurTransactionContext needs to be re-used further and kept it
around. We now use a proper memory context to avoid this.

Fixes timescale#7053
  • Loading branch information
nikkhils committed Jul 2, 2024
1 parent 1b7f109 commit 455c157
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 1 deletion.
2 changes: 2 additions & 0 deletions .unreleased/fix_7088
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixes: #7088 Fix leaks with functions in DML
Thanks: @Kazmirchuk for reporting this
10 changes: 9 additions & 1 deletion src/cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,16 @@ release_subtxn_pinned_caches(SubTransactionId subtxnid, bool abort)
{
ListCell *lc;

/* Need a copy because cache_release will modify pinned_caches */
/*
* Need a copy because cache_release will modify pinned_caches.
*
* This needs to be allocated in pinned cache memory context.
* Otherwise leaks ensue if CurTransactionContext (which is the
* CurrentMemoryContext) gets used!
*/
MemoryContext old = MemoryContextSwitchTo(pinned_caches_mctx);
List *pinned_caches_copy = list_copy(pinned_caches);
MemoryContextSwitchTo(old);

/* Only release caches created in subtxn */
foreach (lc, pinned_caches_copy)
Expand Down
37 changes: 37 additions & 0 deletions test/expected/copy_memory_usage.out
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,40 @@ select * from portal_memory_log where (
----+-------
(0 rows)

-- Test plpgsql leaks
CREATE TABLE test_ht(tm timestamptz, val float8);
SELECT * FROM create_hypertable('test_ht', 'tm');
NOTICE: adding not-null constraint to column "tm"
hypertable_id | schema_name | table_name | created
---------------+-------------+------------+---------
2 | public | test_ht | t
(1 row)

-- Use a plpgsql function to insert into the hypertable
CREATE OR REPLACE FUNCTION to_double(_in text, INOUT _out double precision)
LANGUAGE plpgsql IMMUTABLE parallel safe
AS $$
BEGIN
SELECT CAST(_in AS double precision) INTO _out;
EXCEPTION WHEN others THEN
--do nothing: _out already carries default
END;
$$;
-- TopTransactionContext usage needs to remain the same after every insert
-- There was a leak earlier in the child CurTransactionContext
BEGIN;
INSERT INTO test_ht VALUES ('1980-01-01 00:00:00-00', to_double('23.11', 0));
SELECT sum(total_bytes) from pg_backend_memory_contexts where parent = 'TopTransactionContext';
sum
-------
16384
(1 row)

INSERT INTO test_ht VALUES ('1980-02-01 00:00:00-00', to_double('24.11', 0));
SELECT sum(total_bytes) from pg_backend_memory_contexts where parent = 'TopTransactionContext';
sum
-------
16384
(1 row)

COMMIT;
22 changes: 22 additions & 0 deletions test/sql/copy_memory_usage.sql
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,25 @@ select * from portal_memory_log where (
from portal_memory_log
);

-- Test plpgsql leaks
CREATE TABLE test_ht(tm timestamptz, val float8);
SELECT * FROM create_hypertable('test_ht', 'tm');
-- Use a plpgsql function to insert into the hypertable
CREATE OR REPLACE FUNCTION to_double(_in text, INOUT _out double precision)
LANGUAGE plpgsql IMMUTABLE parallel safe
AS $$
BEGIN
SELECT CAST(_in AS double precision) INTO _out;
EXCEPTION WHEN others THEN
--do nothing: _out already carries default
END;
$$;

-- TopTransactionContext usage needs to remain the same after every insert
-- There was a leak earlier in the child CurTransactionContext
BEGIN;
INSERT INTO test_ht VALUES ('1980-01-01 00:00:00-00', to_double('23.11', 0));
SELECT sum(total_bytes) from pg_backend_memory_contexts where parent = 'TopTransactionContext';
INSERT INTO test_ht VALUES ('1980-02-01 00:00:00-00', to_double('24.11', 0));
SELECT sum(total_bytes) from pg_backend_memory_contexts where parent = 'TopTransactionContext';
COMMIT;

0 comments on commit 455c157

Please sign in to comment.