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 #7053

(cherry picked from commit ebbca2d)
  • Loading branch information
nikkhils authored and timescale-automation committed Jul 2, 2024
1 parent 8c3de81 commit 4eef9d2
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 2 deletions.
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;
4 changes: 3 additions & 1 deletion test/sql/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ if(CMAKE_BUILD_TYPE MATCHES Debug)
TEST_FILES
bgw_launcher.sql
c_unit_tests.sql
copy_memory_usage.sql
metadata.sql
multi_transaction_index.sql
net.sql
Expand All @@ -112,6 +111,9 @@ if(CMAKE_BUILD_TYPE MATCHES Debug)
if(USE_TELEMETRY)
list(APPEND TEST_FILES telemetry.sql)
endif()
if((${PG_VERSION_MAJOR} GREATER_EQUAL "14"))
list(APPEND TEST_FILES copy_memory_usage.sql)
endif()
endif(CMAKE_BUILD_TYPE MATCHES Debug)

if((${PG_VERSION_MAJOR} GREATER_EQUAL "13"))
Expand Down
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 4eef9d2

Please sign in to comment.