whitfin / cachex

A powerful caching library for Elixir with support for transactions, fallbacks and expirations
https://hexdocs.pm/cachex/
MIT License
1.59k stars 102 forks source link

Blocking Preemptive Warmer #242

Closed sneako closed 4 years ago

sneako commented 4 years ago

I have an application in which I store some externally managed configuration in Cachex. Without this configuration, the application is completely unusable, and should not accept any reqeuests.

I use a Preemptive Warmer to hydrate this cache, but since the warmer is executed asynchronously, my application finishes booting, and begins accepting requests before the warmer has finished executing.

Would you be open to a PR adding an option to use Cachex.Warmer, maybe something like block_on_startup: true, which would cause the execute callback to be executed during init?

Thanks for the amazing library!

sneako commented 4 years ago

Here is one potential implementation: https://github.com/whitfin/cachex/compare/master...sneako:blocking-warmer

dch commented 4 years ago

Nice. I would definitely use this. As a suggestion, the handle_continue callback might be a better choice for what you need to do here. IIRC this is in OTP21+ so bear that in mind.

If the gen_server process needs to perform an action immediately after initialization or to break the execution of a callback into multiple steps, it can return {continue,Continue} in place of the time-out or hibernation value, which will immediately invoke the handle_continue/2 callback.

dch commented 4 years ago

send it as a PR @sneako

sneako commented 4 years ago

Nice. I would definitely use this. As a suggestion, the handle_continue callback might be a better choice for what you need to do here. IIRC this is in OTP21+ so bear that in mind.

I agree, for the default behaviour, handle_continue is the way to go. I will happily add that to the PR or a subsequent PR if we are ok dropping support for older OTP versions, but currently the build matrix includes OTP 18+

whitfin commented 4 years ago

This looks ok... but I'm now wondering whether this should just be the default. Do you think this is something that would trip people up often?