Skip to content

fix: minor improvements (xact callback leak, pg_net.ttl validation) - #278

Open
utkarash2991 wants to merge 3 commits into
masterfrom
fix/ttl-validation-and-xact-callback
Open

fix: minor improvements (xact callback leak, pg_net.ttl validation)#278
utkarash2991 wants to merge 3 commits into
masterfrom
fix/ttl-validation-and-xact-callback

Conversation

@utkarash2991

Copy link
Copy Markdown
Contributor

Minor improvements in pg_net:

  • Register the wake_at_commit transaction callback once per backend instead of once per transaction. Fixes a per-backend leak in TopMemoryContext and ever-slower CallXactCallbacks on long-lived connections. Measured 200,120 bytes of growth over 5000 transactions before, 0 after.
  • Validate pg_net.ttl when it is set: invalid and negative intervals are rejected at ALTER SYSTEM time with the interval parser's message, instead of being accepted and failing inside the worker. Closes Validate ttl config #268.

Tested on PG 13, 17 (full suite) and 19.

Tracking: PSQL-1652

`net.wake()` called `RegisterXactCallback(wake_at_commit)` in every
transaction where `wake_commit_cb_active` was false, i.e. once per
transaction that uses pg_net. `RegisterXactCallback` appends a new entry
to a backend-wide list in TopMemoryContext on every call and never
deduplicates, and nothing ever unregistered it.

On a long-lived backend (e.g. behind a connection pooler) that fires
webhooks continuously this leaks ~40 bytes per transaction and makes
`CallXactCallbacks` walk an ever-growing list on every transaction
event, for every transaction on that backend.

Register the callback once per backend and keep
`wake_commit_cb_active` as the per-transaction gate.

The new test measures TopMemoryContext of a backend across 5000
single-statement transactions calling `net.wake()`. Measured on PG 17:
200,120 bytes of growth before this fix, 0 bytes after. Skipped on
PG < 14 where `pg_backend_memory_contexts` doesn't exist.
Add a GUC check hook for `pg_net.ttl` that parses the value with
`interval_in` and rejects invalid and negative intervals at
SET/ALTER SYSTEM/config-reload time, with the interval parser's own
message as the error detail. Previously an invalid value was accepted
and only failed later inside the worker's expired-response delete.

Closes #268.

@imor imor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test asserts can be improved a bit, otherwise looks good.

Comment thread test/test_ttl_config.py Outdated
autocommit_sess.execute(text(f"alter system set pg_net.ttl to '{bad}'"))
msg = str(excinfo.value)
assert 'invalid value for parameter "pg_net.ttl"' in msg
assert "invalid input syntax for type interval" in msg

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we assert the value of the full error message, that will make it easy to see the complete error we expect users to see as well.

Check the exact primary message and DETAIL line via the driver's
diagnostics instead of two substrings, so the test documents the
complete error a user sees on ALTER SYSTEM.
@utkarash2991
utkarash2991 force-pushed the fix/ttl-validation-and-xact-callback branch from 7da0030 to 40d93d0 Compare September 8, 2026 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validate ttl config

2 participants