Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non transactional cache hit/miss statistics #308

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ddorian
Copy link
Contributor

@ddorian ddorian commented Mar 4, 2024

@grantjenks

This is to move cache hit/miss outside of the get transaction.

A use case is to override it, make it async, and update the hit/miss counters in the db every 2-seconds instead of every transaction.

If you agree I'll fix linting etc.

@grantjenks
Copy link
Owner

A use case is to override it, make it async, and update the hit/miss counters in the db every 2-seconds instead of every transaction.

I don’t see why these changes are necessary for this use cases. I think you could track hits/misses outside the cache already and update a different key (or cache) with the information asynchronously. Leveraging the cache’s Settings metadata seems like a mistake.

@ddorian
Copy link
Contributor Author

ddorian commented Mar 4, 2024

I don’t see why these changes are necessary for this use cases.

The first use case is even the default implementation to not be transactional (less locking when having statistics and reading from external files).

The other idea is I'd have to add another approach (can't reuse self.statistics, have to check the returned value in get() and compare it against default, etc).

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.

2 participants