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

Plans for an IDistributedCache implementation? #85

Open
jodydonetti opened this issue Mar 22, 2024 · 9 comments
Open

Plans for an IDistributedCache implementation? #85

jodydonetti opened this issue Mar 22, 2024 · 9 comments
Assignees
Labels
enhancement New feature or request

Comments

@jodydonetti
Copy link

Hi all, are you planning on creating an IDistributedCache implementation anytime soon?

Since Garnet has been announced as being protocol-compatible with Redis I thought it would've worked right away by using the standard Redis implementation (see here), but after actually playing with it I discovered that Garnet does not currently support LUA scripting:

  1. Garnet does not support Lua scripting. We have an experimental version, but it was noted to be too slow for realistic use so we have not added it to the project.

LUA scripting is what has been used in the aforementioned impl (see here) and therefore without it it's not possible to use Garnet this way.

Having a working impl of IDistributedCache for Garnet would drive the adoption a lot, since that is the standard way of using a distributed cache in .NET.

Also, that would unlock other great possibilities like being able to use Garnet in projects that use IDistributedCache as a "lingua franca", like FusionCache (mine) which is compatible with any impl of such interface (here for more).

All in all I think that, if the effort for such impl is not huge, that is something that you should consider to skyrocket the usage.

Thoughts?

ps: maybe @mgravell can give us some hints about such approach? Are there any hidden pitfalls that should be looked at?

@jodydonetti jodydonetti changed the title Plan for an IDistributedCache implementation? Plans for an IDistributedCache implementation? Mar 22, 2024
@mgravell
Copy link
Contributor

mgravell commented Mar 22, 2024

Thanks @jodydonetti - great spot!

I don't know anything about the Garnet side of things (I'm involved in caching on behalf of the asp.net team); I'm going to assume no immediate changes to Garnet to support Lua or something like redis "modules" (i.e. loadable server-side commands) is possible; happy to be corrected.

But building on that assumption: I think this is more of a question for me, over in https://github.com/dotnet/aspnetcore; IMO what we should do here is ... (reads the code)... you know, reading that code, there's nothing there that needs Lua; the "get and extend sliding" would have been a good fit for Lua, but: we don't use Lua for that - we only use it for the set path, with code that doesn't actually depend on the existing DB state; frankly, we could just multi/exec or F+F pipeline the H[M]SET and EXPIRE. Let's just do that! I think what has happened here is that the code has iterated and what once made sense as Lua: now doesn't.


On unrelated Garnet-ish : I've been speaking to the FASTER folks about whether we should also have a FASTER cache for disk-based (non-server) local cache via IDistributedCache; if we do this, current thinking is that for now this might be a "labs" thing, i.e. a lower support thing, but it would help gauge interest.

@mgravell
Copy link
Contributor

I've created a ticket "over there ^^^" - can someone confirm whether Garnet has MULTI/EXEC support? this may influence any changes there

@PaulusParssinen
Copy link
Contributor

... can someone confirm whether Garnet has MULTI/EXEC support? this may influence any changes there

Quickly looking at the code: yes, it support MULTI/EXEC.

@badrishc
Copy link
Contributor

badrishc commented Mar 22, 2024

Garnet supports C# server side transactional stored procedures which are way superior to Lua scripting, plus thread scalable. Will be easy if we could make ASP.NET use that. Stored procs are accessed via custom commands - DB.Execute in SE.Redis.

And yes, we have MULTI/EXEC as well. That should be thread-scalable too. Maybe a tad bit slower. Would be interesting to compare performance.

@badrishc
Copy link
Contributor

More info here: https://microsoft.github.io/garnet/docs/extensions/transactions

@mgravell
Copy link
Contributor

mgravell commented Mar 22, 2024

I ended up just switching it to pipeline (no multi/exec) - dotnet/aspnetcore#54689

In theory I'm fine with a custom stored proc approach, but it creates fun new places to fail:

  • the stored proc needs to be written and maintained by someone
  • it needs to be installed on the server
  • it needs an opt-in flag at the client to know about them (since that won't work with non-Garnet backends)

By contrast, when simply pipelining, the exact same code should JustWork™ everywhere

@jodydonetti
Copy link
Author

jodydonetti commented Mar 22, 2024

My 2 cents on the overall approach: if by slightly changing the standard Redis impl to avoid using LUA scripts leads to better perf "this is the way". If that in turn also leads to be compatible with Garnet, even better.

On top of that, if (and only if) it makes sense to better use specific features of Garnet and get even more perf (and maybe also add some extra features on the concrete impl?) then a Garnet-specific impl of IDistributedCache may be created.

Users can then either:

  • pick the Garnet-specific one if they want absolutely maximum performance (or the potential extra features)
  • pick the standard Redis one if they prefer to use a generic Redis client to switch between Redis and Garnet for whatever reason (local vs tests vs prod?)

@tfranzon
Copy link

tfranzon commented May 3, 2024

@mgravell
We are using .NET Framework 4.8 and IDistributedCache and gets the "ERR unknown command". Is there any plans to fix this for 4.8 too?

@mgravell
Copy link
Contributor

mgravell commented May 3, 2024

Already done in dotnet/aspnetcore#54689; should work from preview4 onwards, and yes that includes backport to older TFMs - we target ".NET current" (so: .NET 9 in preview 4), net462 and ns2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants