Skip to content

feat(config): add query_cache_memory_limit and query_cache_idle_timeout - #1266

Open
IgorOhrimenko wants to merge 3 commits into
pgdogdev:mainfrom
IgorOhrimenko:query-cache-memory-limit-ttl
Open

feat(config): add query_cache_memory_limit and query_cache_idle_timeout#1266
IgorOhrimenko wants to merge 3 commits into
pgdogdev:mainfrom
IgorOhrimenko:query-cache-memory-limit-ttl

Conversation

@IgorOhrimenko

@IgorOhrimenko IgorOhrimenko commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Implements the memory-based cache limit and idle expiry proposed in #1261.

Adds two [general] options (also PGDOG_QUERY_CACHE_MEMORY_LIMIT / PGDOG_QUERY_CACHE_IDLE_TIMEOUT env), both default 0 (off):

  • query_cache_memory_limit (bytes): evict LRU until the summed entry size is under the budget. query_cache_limit bounds the number of entries but not their memory — a parsed tree for a wide/complex query is orders of magnitude larger than for SELECT 1, so a count-based cap alone can't bound RAM. Entry size is measured via jemalloc's per-thread allocation counters (clamped to 0, since cross-thread frees under a work-stealing runtime can make the delta negative); falls back to the query text length on non-jemalloc builds.
  • query_cache_idle_timeout (milliseconds): entries not accessed within the window are dropped by the existing 1s maintenance sweep, releasing an idle working set without waiting for LRU eviction.

Both are opt-in (0 = off); default behaviour is unchanged. Unit tests cover count/byte eviction, byte accounting and the idle-expiry sweep; config parsing is covered in pgdog-config. JSON schema regenerated.

Closes #1261.

The AST query cache was bounded only by entry count (query_cache_limit),
not by memory, and had no idle expiry. Heavy/complex queries produce large
parse trees, so a full cache can hold GBs of live RSS that only drop once
1000 newer distinct queries evict them (or on RESET/restart).

Adds two [general] options (also PGDOG_QUERY_CACHE_MEMORY_LIMIT /
PGDOG_QUERY_CACHE_TTL env), both default 0 (off):

- query_cache_memory_limit: evict LRU until the summed entry size is under
  the byte budget. Entry size is measured via jemalloc per-thread allocation
  counters (clamped to 0, since cross-thread frees under a work-stealing
  runtime can make the delta negative); falls back to query length on
  non-jemalloc builds.
- query_cache_ttl: drop entries not accessed within the window, via the
  existing 1s maintenance sweep.

Refs pgdogdev#1261.
@IgorOhrimenko

Copy link
Copy Markdown
Contributor Author

@levkk — a direction check on this one.

The failure mode this PR targets: the query cache is bounded by entry count but not by bytes, and entries never expire. On a workload with large or diverse queries, RSS grows monotonically until the container hits its memory limit — at which point the kernel OOM killer SIGKILLs the process. No drain, no graceful shutdown, exit 137: every client connection is dropped mid-transaction, and it tends to happen at peak traffic, because that's when the cache churns hardest. For a connection pooler — the component that exists to make everything behind it look stable — that's the worst possible way to die.

Both knobs are opt-in (0 = off), so default behaviour is byte-for-byte unchanged. What they buy is turning "hard kill at an unpredictable time" into a bounded steady state the operator chooses: query_cache_memory_limit caps the total, query_cache_ttl releases an idle working set without waiting for eviction.

… use milliseconds

The behaviour is time-to-idle (expiry counted from last access), not
time-to-live, so the ttl name over-promised. The config already has an
idle-timeout vocabulary (idle_timeout, client_idle_timeout) — reuse it,
and switch the unit from seconds to milliseconds to match the other
*_timeout options. Env variable becomes PGDOG_QUERY_CACHE_IDLE_TIMEOUT.
Schema regenerated.
@IgorOhrimenko IgorOhrimenko changed the title feat(config): add query_cache_memory_limit and query_cache_ttl feat(config): add query_cache_memory_limit and query_cache_idle_timeout Aug 1, 2026
@IgorOhrimenko

Copy link
Copy Markdown
Contributor Author

Renamed query_cache_ttlquery_cache_idle_timeout (b78543c) after a closer look at the naming.

What the option implements is time-to-idle, not time-to-live: the clock counts from the last access, so a hot entry never expires — only entries nobody has touched for the whole window are swept. A true TTL counts from insertion and would evict hot entries too, which is the wrong tool here (a parsed AST can't go stale, so there's nothing to refresh). Calling a time-to-idle knob ttl would over-promise expiry that never happens for hot entries.

The precise term for this is TTI (timeToIdle in Ehcache, AccessedExpiryPolicy in JCache, expireAfterAccess in Caffeine), but it's obscure outside the Java caching world — and the config already has an established vocabulary for exactly this semantic: idle_timeout, client_idle_timeout. So the option now follows it: query_cache_idle_timeout, and the unit changed from seconds to milliseconds to match the other *_timeout options. Env var is now PGDOG_QUERY_CACHE_IDLE_TIMEOUT; schema regenerated; PR description updated.

IgorOhrimenko added a commit to IgorOhrimenko/docs that referenced this pull request Aug 1, 2026
…lliseconds)

Follows the rename in pgdogdev/pgdog#1266: the behaviour is idle
expiry (counted from last access), so the option reuses the existing
idle-timeout naming and the millisecond unit of the other *_timeout
options.
@codecov

codecov Bot commented Aug 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.27798% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...dog/src/frontend/router/parser/cache/cache_impl.rs 99.17% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

The existing tests exercise the limit logic on a standalone Inner, so the
public entry points that apply configuration to the global cache were
never called: configure() with a non-zero idle timeout and resize()
(flagged by codecov on the patch). Cover both, including the zero-capacity
floor of resize().
@IgorOhrimenko

Copy link
Copy Markdown
Contributor Author

A note on the Codecov report: the two remaining uncovered lines in cache_impl.rs are defensive branches that aren't reachable through the public API.

  • Line 101 — the None => break arm in enforce(). It guards against an infinite eviction loop if the byte accounting ever drifted out of sync with an empty cache. With insert/sweep keeping bytes consistent, pop_lru() can't return None while a limit is still violated.
  • Line 154 — the fallback in measure_build() for when jemalloc's per-thread allocation counters are unavailable at runtime. On builds where jemalloc is active (including the coverage CI build), the Some path is always taken.

Everything else in the patch, including configure() with a non-zero idle timeout and resize(), is now covered.

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.

Query cache (query_cache_limit) is bounded by entry count, not memory, and has no TTL — heavy queries balloon RSS until restart

1 participant