Skip to content

Enforce prepared statements cache limits at runtime, add memory limit - #1311

Draft
IgorOhrimenko wants to merge 7 commits into
pgdogdev:mainfrom
IgorOhrimenko:prepared-statements-memory-limit
Draft

Enforce prepared statements cache limits at runtime, add memory limit#1311
IgorOhrimenko wants to merge 7 commits into
pgdogdev:mainfrom
IgorOhrimenko:prepared-statements-memory-limit

Conversation

@IgorOhrimenko

@IgorOhrimenko IgorOhrimenko commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Problem

The global prepared statements cache is enforced by a 1 Hz maintenance task that calls close_unused(prepared_statements_limit), but three things undermine it:

  • The default prepared_statements_limit is i64::MAX, so out of the box every statement a client ever released is kept forever.
  • There is no memory-based limit at all: the count says nothing about entry size, and entries carry the full Parse (plus a rewritten copy and a RowDescription when present).
  • Enforcement only happens once a second, so a burst of prepare/close cycles can overshoot between ticks.

What this PR does not fix, explicitly: statements a client is still holding can never be evicted — dropping one would break the client using it. A workload that keeps millions of unique statements open (we watched ~6M held statements take a pooler to ~7 GiB RSS until the clients disconnected) is out of reach for any unused-only eviction, including this one and the existing maintenance task. Protecting against that needs a per-client cap or similar — happy to discuss as a follow-up; this PR bounds what released statements can accumulate, immediately rather than at the next tick.

Change

Same shape as the query cache limits in #1266:

  • prepared_statements_limit and the new prepared_statements_memory_limit (bytes, 0 = unlimited, env PGDOG_PREPARED_STATEMENTS_MEMORY_LIMIT, settable from the admin console) are enforced at the moment the cache grows or a statement is released, not just at the next maintenance tick.
  • Eviction is deterministic — oldest released statement first (unused is a BTreeSet now) — and only ever touches statements nobody holds.
  • The byte total is maintained incrementally on insert/remove/rewrite/describe, so enforcement doesn't rescan the maps, and GlobalCache::memory_usage() returns that same number — the prepared_statements_memory_used metric and the budget can't drift apart. A prepared_statements_memory_limit gauge is exported next to it.

Behavior changes

  • Admin console SET prepared_statements_limit TO 0 used to wipe the cache; it now means "unlimited" (matching the config semantics) and logs a warning pointing at RESET PREPARED.
  • RESET PREPARED now passes 0 explicitly and clears everything not in use regardless of the configured limit — with the default (unlimited) limit it used to be a no-op.
  • close_unused(0) no longer resets the name counter: global __pgdog_N names are never reused, so a server connection holding an old name can't be handed a different query under it.

Testing

Unit tests for enforcement (capacity and memory eviction, in-use statements never evicted, 0 = unlimited, immediate enforcement on configure, deterministic oldest-first order, release via both close() and decrement(), counter preserved across RESET PREPARED) and a byte-accounting invariant test that recounts the total from live entries after a mix of inserts, duplicate inserts, insert_anyway, late RowDescription, rewrite() (including replacement), closes and evictions. Config env/default tests; JSON schema regenerated.

Cost note

Eviction runs under the global cache write lock. The maintenance task now goes through configure() once a second too, but when the cache is within its limits that's a couple of integer comparisons — there is nothing to evict, because insert/release enforcement already kept it in budget. The one expensive case is lowering a limit (admin SET, RELOAD) over a cache holding millions of released statements: they are all evicted in one pass and cache users wait for the duration. That's a one-off administrative action; chunked eviction can be added if it matters in practice.

Docs PR to follow once the shape is agreed on.

The cache had no working limit: prepared_statements_limit was applied
to it only on RELOAD and from the admin console, so between those a
workload preparing a stream of unique statements grows it without
bound. A statement spike of a few million unique queries takes a
pooler to gigabytes of RSS, and nothing reclaims that while traffic
keeps flowing.

Enforce the count limit continuously and add a companion
prepared_statements_memory_limit (bytes, 0 = unlimited, also
PGDOG_PREPARED_STATEMENTS_MEMORY_LIMIT and settable from the admin
console), the same shape as the query cache limits. Only statements no
client is holding are evicted, so the cache can still exceed its caps
while everything in it is in use — it shrinks the moment statements
are released. The byte total is maintained incrementally on
insert/remove, so enforcement doesn't rescan the maps; the same number
feeds the new prepared_statements_memory_limit gauge next to the
existing prepared_statements_memory_used.

One behavior change in the admin console: SET
prepared_statements_limit TO 0 used to wipe the cache (close_unused
treats 0 as "remove everything"); it now means "unlimited", matching
the config semantics.
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.59677% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...g/src/frontend/prepared_statements/global_cache.rs 99.53% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@IgorOhrimenko

Copy link
Copy Markdown
Contributor Author

On the Codecov report: the one uncovered line in the patch is the evict_on_close arm in close() (self.remove(name) at global_cache.rs:364). The edit next to it pulled the arm into the patch, but the flag itself is never set to true anywhere in the codebase today, so the branch is unreachable — no test can cover it. Happy to drop the field and the arm here or in a follow-up if it's actually dead, or leave it alone if it's groundwork for something planned.

The unused set was a hash set, so eviction order depended on hasher
state. A BTreeSet keyed by the statement counter makes it evict the
oldest statement first, deterministically, and close_unused inherits
the same order.

SET prepared_statements_limit TO 0 used to clear the cache and now
means unlimited, so log a warning pointing at RESET prepared_statements
for operators relying on the old behavior.
- close_unused(0) no longer wipes the whole cache and resets the name
  counter: it now drops everything not in use, keeps statements clients
  hold, and never reuses global names. The old reset path could hand a
  server connection a reused __pgdog_N name pointing at a different
  query.
- RESET prepared_statements passes 0 explicitly, so it clears the cache
  regardless of the configured limit. With the default (unlimited)
  limit it was a no-op.
- GlobalCache::memory_usage() now returns the same number the memory
  limit is enforced against, so the prepared_statements_memory_used
  metric and the budget can't drift apart.
- Statement::memory_usage() counts the rewritten Parse; rewrite() and
  insert_row_description() adjust the byte total and enforce.
- remove() clears the unused entry for evict_on_close closes and
  asserts the two maps stay in sync; enforce() uses pop_first().
- Config doc note: a limit below the working set causes constant
  re-preparation.
@IgorOhrimenko

IgorOhrimenko commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

A note on the ci (complex) failures on the latest commit (three in a row now): it's the shutdown check timing out, not a regression.

  • The failing runs stop at waiting on N background tasks and hit shutdown.sh's 10-second budget; the ci-new-parser twin of the same suite on the same SHA passes every time.
  • Locally the suite passes 7/7 on this commit unconstrained. Pinned to a single CPU to simulate a busy runner, the shutdown check fails on both this commit and its parent (which passed CI three times) at the same rate — 1/8 each — with the same waiting on N background tasks signature.
  • On a healthy runner the drain takes ~5 of the allowed 10 seconds (340 → 304 → 23 tasks over ~5s in the passing log), so there's no headroom when the runner is loaded.

Same class of timing flake as #1303/#1304/#1305. Bumping wait_for_shutdown in integration/complex/shutdown.sh from 100 to ~300 iterations would give the drain the headroom it needs; happy to include that here or send it separately.

run_maintenance() passed prepared_statements_limit straight into
close_unused(), where 0 now means "drop everything unused" — the exact
opposite of the "unlimited" this limit documents. Re-apply the
configured limits through configure() instead: 0 flows through
over_budget() as unlimited, the memory limit gets the same safety net,
and the maintenance tick stays a no-op when runtime enforcement has
already done the work.

GlobalCache::reset() lost its last production caller when close_unused
stopped wiping the cache; keep it for tests only, so the path that
rolls the name counter back can't quietly return.
The admin parser accepts RESET PREPARED, not RESET prepared_statements:
the warning was advising a command that answers with a syntax error.
Verified against a live admin console.
The limit-0 warning quoted a command spelling the admin parser doesn't
accept; nothing tied the two together. Put the spelling in one const —
ResetPrepared::name(), the warning and the parser test all use it — so
the advice can't drift from what actually parses. Also covers RESET
PREPARED in the parser tests at all: RESET QUERY_CACHE had a test, this
one didn't.
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.

1 participant