fix(memory): prepared statements cache retains gigabytes after statement spike - #1282
fix(memory): prepared statements cache retains gigabytes after statement spike#1282IgorOhrimenko wants to merge 8 commits into
Conversation
…ccounting The global prepared statements cache reports memory usage by summing live entries only. Hash tables allocate capacity, not len: after a spike of unique prepared statements the spare capacity left behind dominates actual memory use but is invisible to the metric, which reports near zero while the process holds gigabytes. Count allocated slots (plus control bytes) in the MemoryUsage impls for HashMap and HashSet, make them generic over the hasher so FnvHashSet is covered, and export the statements table capacity as a new prepared_statements_capacity gauge: a capacity far above prepared_statements signals memory held from a past spike.
Hash tables never return capacity to the allocator: after a spike of unique prepared statements from long-lived clients, the global cache tables keep gigabytes of empty buckets for the lifetime of the process. Shrink the tables in the maintenance sweep once they are mostly empty (capacity > 8x len) and large enough to matter (> 4096 slots), leaving 2x headroom to avoid rehashing on every sweep. Also shrink on reset().
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…ise metric help The MemoryUsage trait mixes inline-size scalars with heap-counting containers; state explicitly that aggregated numbers are upper-bound approximations. Compare capacity against len * factor directly instead of integer division, and say which table the capacity gauge measures.
| use std::collections::{BTreeMap, HashMap, HashSet, VecDeque}; | ||
| use std::hash::Hash; | ||
|
|
||
| /// Approximate bytes attributable to a value, for metrics. |
There was a problem hiding this comment.
A note on the accounting precision, since it is not obvious from the diff:
MemoryUsage impls are mixed — scalars and small structs (usize, CacheKey, CachedStmt) report their inline size, while String/BytesMut report heap capacity. With the new capacity term in the HashMap/HashSet impls this means the inline portion of live entries is counted twice: once inside the table's capacity, once in the per-element sum.
The overcount is bounded by len * inline_size, which matters only when the table is full (a few hundred MB at millions of entries, against multi-GiB actual usage) and vanishes in the post-spike state this metric is meant to catch (few live entries, huge capacity). Making it exact would require splitting the trait into inline/heap halves across every impl, which does not seem worth it for an observability gauge — the doc comment on the trait now states the upper-bound semantics.
| + self.counter.memory_usage() | ||
| + self.versions.memory_usage() | ||
| + self.unused.len() * std::mem::size_of::<usize>() | ||
| + self.unused.memory_usage() |
There was a problem hiding this comment.
I remember removing this because it was very expensive to calculate (O(n)).
| /// statements. Hysteresis (mostly-empty table, above a minimum size) | ||
| /// avoids rehashing on every sweep; shrinking to twice the current | ||
| /// len leaves headroom for new statements. | ||
| fn maybe_shrink(&mut self) { |
There was a problem hiding this comment.
Why not call shrink_to_fit instead? https://doc.rust-lang.org/stable/std/collections/struct.HashMap.html#method.shrink_to_fit
| // The table allocates capacity() slots (plus one control byte each), | ||
| // not len(): spare capacity left behind by removed entries still | ||
| // occupies memory and has to be counted. | ||
| self.capacity() * (std::mem::size_of::<(K, V)>() + 1) |
There was a problem hiding this comment.
This is O(n). We should be careful not to call this frequently. I had to debug CPU usage issues with this before.
rustfmt splits macro calls longer than its small-heuristics width, which puts assert messages back on their own never-executed lines; the values are in named locals right above, so the messages add nothing.
2 participants
Problem
Long-lived clients that keep preparing unique SQL statements (report jobs, ORMs interpolating values into SQL) grow the global prepared statements cache to millions of entries. When they disconnect, the maintenance sweep correctly trims entries down to
prepared_statements_limit— but the process keeps holding gigabytes of RSS indefinitely:HashMap/HashSetnever return capacity to the allocator. After a spike of 10M statements, the three tables inGlobalCache(statements,names,unused) keep ~13.5M slots (~5.9 GiB) at 100 live entries, forever.prepared_statements_memory_usedis blind to this:MemoryUsage for HashMapsums live entries viaiter(), so it reports ~44 KiB while RSS holds ~10 GiB.Reproduction (docker compose + load generator): https://gist.github.com/IgorOhrimenko/8ba71664c7cf27f005e9ef80a48cf97c
Measured on a synthetic load test (10M unique prepared statements at ~7k/s through one PgDog instance, then all clients disconnect; all values settled):
memory_usedafter disconnectcapacityafter disconnectChanges
fix(stats): count hash table capacity in memory accountingMemoryUsage for HashMap/HashSetnow counts allocated slots (plus control bytes) and is generic over the hasher, soFnvHashSetis covered.prepared_statements_capacitygauge:capacityfar aboveprepared_statementssignals memory held from a past spike, which makes this failure mode visible on a dashboard.fix(memory): shrink the tables in the maintenance sweepclose_unused()callsshrink_to(len * 2)on all three tables once they are mostly empty (capacity > 8 * len) and large enough to matter (> 4096slots). Hysteresis avoids rehashing on every sweep; with jemalloc'sbackground_thread(default since fix(memory): enable jemalloc background_thread by default #1260) the freed pages are returned to the OS within seconds.reset()also releases capacity.Peak memory under load is unchanged — entries held by connected clients are legitimate cache content; this PR is about returning the memory after they are gone.
Testing
cargo nextest run --test-threads=1over the workspace packages against a local Postgres — green.