-
Notifications
You must be signed in to change notification settings - Fork 73
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
Hit/miss ratio tracking #75
Comments
Sorry for the late reply, I was a bit busy, and also wanted to think this through. My first reaction was that this can be done by the client on top of the already existing interface. And while that is true, it's not completely obvious how, plus it requires some boilerplate, and it's possible to mess it up. So I think it makes sense to implement this in con_cache. Do you have a proposal approach? |
Hey,
That's what I did and it indeed wasn't very trivial.
I think emitting telemetry events for hit/miss would be enough for users to integrate as opt-in. |
I like it. It should require a small amount of changes to con_cache. Plus it's very flexible. Out of curiosity, what's your use-case for this and how does your implementation work? I wonder if we could also add a simple implementation, e.g. based on :counters. |
OK so the context is: we have several different caches in https://github.com/plausible/analytics/ - we like to understand utilization of each (expose charts via grafana/prometheus). The way it's currently implemented: there is an adapter that interfaces with I did consider atomic counters for this, but ended up using I think providing a |
I forgot to add, historically it was Cachex before ConCache, hence the adapter dance - to keep the existing stuff relatively intact etc. |
I think you could use the high-level API, if in your own
Yeah, I think I see what you mean, good point!
How come you're using a global in-memory state for this? Wouldn't it make more sense to forward hit/miss events to the external metrics collector? That way you can aggregate data from multiple instances, and see moving averages over shorter time periods (e.g. minute), rather than the total absolute since the last deploy. |
You're right, we're currently winding down everything to almost 0 as the app nodes rotate, this was never a concern though. I suppose this is very specific to our use case. The peak is always somewhat the same, greater precision would get us very little value at the cost of having to maintain yet another moving part (the external collector). Anyway, would you like me to try and implement the telemetry events emission? With or without the stats collector? |
Yeah, let's do this. Let's start with just telemetry events then, so basically the bare minimum that supports your use case. |
PR up, happy to discuss and re-iterate. |
closed via #76 |
Hey, thanks for this library.
Would you be interested in an extension allowing querying each cache's hit rate? Happy to discuss implementation details if you're open to the idea.
The text was updated successfully, but these errors were encountered: