Add Split::Cache to reduce load on Redis

[rdh]

The Split.configuration.cache boolean value will enable in memory caching of infrequently changing values from Redis. Changes to these values will not be reflected until restart, or when Split::Cache.clear is called.

[amangup]

I think for merging it to main branch, we need to make a few more changes, but for us it works (except the winner caching). I will test it out.

[rdh]

I think for merging it to main branch, we need to make a few more changes, but for us it works (except the winner caching). I will test it out.

Yeah. I'm not sure about disable_metrics either. In our world, saw a lot of read-only access, but I think that may not be appropriate for wider usage. May back that one out and eat the Redis spam.

[andrehjr]

Thanks for working on this ✨

I haven't tested this fully, but I like where this is going.

I have some plans to reduce/refactor some calls on split codebase to reduce the amount of Redis calls. But until that happens, having this cache around would be nice.

[rdh]

Thanks for working on this ✨

I haven't tested this fully, but I like where this is going.

I have some plans to reduce/refactor some calls on split codebase to reduce the amount of Redis calls. But until that happens, having this cache around would be nice.

Happy to contribute. We use your gem like crazy. Have about 50+ experiments and we're bucketing into every experiment on every request, due to some dubious architectural choices.

We just pushed this logic cherry-picked onto 3.4.1 to our production servers today, and our average Redis burn dropped from ~60m to ~30ms. Still too high, but a lot better ;-)

[andrehjr]

Awesome, good to know @rdh ! I wanna release v4 soon, this PR would be a great addition :)

[rdh]

OK. I've added minimal docs, so this is discoverable. I think this PR hangs together, and is already doing good stuff for us. I think it's mergeable "as is" and can convert to real PR, if you're cool with it.

Some possible additions:

1. If you know of any other places that are obvious candidates for caching, I'm happy to wrap them. I just hit the biggest offenders in our app, but suspect there may be other relatively static bits of data we could cache.

2. We discussed adding a TTL for cache entries. I'm fine with "until reboot", and feel like the TTL would add complexity. Would prefer to defer that to a later PR.

3. I feel like the README on this is MVP. Happy to take another swing at wordsmithing, if you'd like a bit more detail?

[andrehjr]

1. If you know of any other places that are obvious candidates for caching, I'm happy to wrap them. I just hit the biggest offenders in our app, but suspect there may be other relatively static bits of data we could cache.

I'll review it fully tomorrow, and I'll let you know if find anything else. I think it's ok to convert to a PR too.

2. We discussed adding a TTL for cache entries. I'm fine with "until reboot", and feel like the TTL would add complexity. Would prefer to defer that to a later PR.

Agreed. It's also disabled by default. We can improve it later

3. I feel like the README on this is MVP. Happy to take another swing at wordsmithing, if you'd like a bit more detail?

I think it's fine for now 👍