Skip to content

feat: make eviction policy configurable and add LFU - #1554

Draft
mehcode wants to merge 4 commits into
pgdogdev:mainfrom
mehcode:rl-lfu-cache-1530
Draft

mehcode wants to merge 4 commits into
pgdogdev:mainfrom
mehcode:rl-lfu-cache-1530

Conversation

@mehcode

@mehcode mehcode commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Closes #1530

  • Add prepared_statements_eviction to [general] taking lru (default) or lfu (also available through PGDOG_PREPARED_STATEMENTS_EVICTION and SET ...)

  • Add use tracking to LocalStatement. The existing ensure_capacity method now branches based on eviction policy. The data structure is still LruCache::unbounded().

  • Make a LocalCache wrapper to extract some of the code from prepared_statements.rs into a separate file.

  • Add several tests that assert behavior of both LRU and LFU policies.

Signed-off-by: Ryan Leckey <leckey.ryan@gmail.com>
Signed-off-by: Ryan Leckey <leckey.ryan@gmail.com>
Signed-off-by: Ryan Leckey <leckey.ryan@gmail.com>
…into its own file

Signed-off-by: Ryan Leckey <leckey.ryan@gmail.com>
@CLAassistant

CLAassistant commented Sep 16, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@codecov

codecov Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codecov Report

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

Files with missing lines Patch % Lines
pgdog-config/src/pooling.rs 83.33% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

closed
}

fn ensure_capacity_lfu(&mut self, count: usize) -> Vec<Close> {

@levkk levkk Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks to be O(n * log * n) runtime and expected(n) memory and runs on each server check-in (once per transaction). This may not work if the number of prepared statements is high, which is common. Why not use a real lfu data structure, e.g. https://docs.rs/lfu/latest/lfu/index.html (don't know if the crate is maintained, just googled it).

Meanwhile LRU is O(1)!

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.

I'd suggest checking this library https://docs.rs/cachekit/0.8.0/cachekit/index.html - because it's a ton of caches and data structures. And it could be even easier if some of the library traits would fit our usecases.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for mentioned this one. That one didn't come up when I searched. I still think a purpose built Cache type is best because we don't need most of what a LRU/LFU crate offers, we explicitly do not want automatic eviction and most libraries are built around providing it.

As far as I can tell, automatic eviction can't be turned off in cachekit. Capacity is allocated up-front so there isn't room for spill over. We can't say "no" when told about a statement coming in.

LfuCache is thread-safe internally when we don't need that for the per connection type. It internally wraps things in Arcs and requires &K not &Q where Q: Borrow<K> which makes working with string keys complicated.

@mehcode
mehcode marked this pull request as draft September 17, 2026 13:38
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.

[Prepared statements] Add LFU cache and make it configurable

4 participants