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

Hits/Misses statistics not updated when using get_all #125

Closed
liamdasilva opened this issue Jun 18, 2021 · 4 comments
Closed

Hits/Misses statistics not updated when using get_all #125

liamdasilva opened this issue Jun 18, 2021 · 4 comments

Comments

@liamdasilva
Copy link

liamdasilva commented Jun 18, 2021

Hi there :)

I have a cache that has statistics enabled with stats: true in the config

config :my_app, MyCache, [
  primary: [
    stats: true,
    # When using :shards as backend
    backend: :shards,
    # GC interval for pushing new generation: 12 hrs
    gc_interval: :timer.hours(12),
    # Max 10,000 entries in cache
    max_size: System.get_env("CACHE_LIMIT", "10000") |> String.to_integer(),
    # Max 2 GB of memory
    allocated_memory: 2_000_000_000,
    # GC min timeout: 10 sec
    gc_cleanup_min_timeout: :timer.seconds(10),
    # GC min timeout: 10 min
    gc_cleanup_max_timeout: :timer.minutes(10)
  ]
]

and with periodic measurements enabled in telemetry

defp periodic_measurements do
    [
      {MyCache, :dispatch_stats, []}
    ]
  end

This produces stats perfectly fine when I use MyCache.get/1, however when using MyCache.get_all/1 I noticed that the hit/miss stats are not updated. Am I doing something wrong or is this a known issue and not supported yet?

Thanks!

@cabol
Copy link
Owner

cabol commented Jun 19, 2021

You're absolutely right, with get_all if a bit thicker because we have to compare which keys were found and which ones weren't, and based on that, update the :hits and :misses. But I will take a look at it. Thanks!

@cabol
Copy link
Owner

cabol commented Jun 19, 2021

Hey @liamdasilva 👋

I found the issue and pushed the fix already to the master branch. Please try it out and let me know if it works for you.

PD: I'll ship a new release soon!

@liamdasilva
Copy link
Author

Wow that was fast, thanks so much Carlos :) I'll test it out and let you know

@liamdasilva
Copy link
Author

Yup, works great, thanks so much :)

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

No branches or pull requests

2 participants