-
Notifications
You must be signed in to change notification settings - Fork 118
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
[BUG] Opaque issue in L2 due to (de)serialization #391
Comments
Hi @gerwim , thanks for the MRE. This is not a bug, you just need to instruct your choosen serializer (in this case NewtonsoftJson) how you want it to handle deserialization of abstract classes. It's not something that FusionCache can know how to do, and not even the serializer itself can know it, you have to configure it in some way. Hope this helps, let me know. PS: one thing I'd like to point out for your convenience is that you don't need a Redis instance to try out stuff like this, you can easily use an instance of |
@jodydonetti I've used Redis as it's a persistent (out of the console app) L2 cache. Because this issue only happens when you restart the application (you need to run it twice, as per "how to reproduce). But I do still think it's a bug or maybe atleast an issue? Why? Let me try to explain: Let's say you have two instances (you are horizontally scaled). I'll call these instances 1 and 2. 1 writes an entry (with an abstract type, as per MRE) to the cache and it works pretty well (as the entry is in the L1 cache). So basically requests to instance 1 will be fine, and requests to instance 2 will throw an exception. This is due how the serialization is done. So I'm not saying it's FusionCache's fault, I'm just saying that the issue is not directly clear because the L1 cache does not suffer from this problem. In my case, I was implementing FusionCache in our solution and everything worked fine. But when deployed to our staging environment, there were these issues. |
Hi @gerwim , I'm commuting now, will answer as soon as I can! |
@jodydonetti No problem, no stress. Take your time. I have to thank you for your time and effort into making this great package. Open source maintainers have no obligation to us whatsoever. 😀 |
Hi @gerwim , sorry for the delay, back on this.
I see your point, and I agree that it would be nice to have some sort of "help" in catching these sorts of errors sooner: for example I did something like this with a warning that is written in the logs when you setup a FusionCache instance with a Backplane but without an L2 and the DefaultEntryOptions are set in a potentially incorrect way. I like these kinds of "quality of life" checks. Having said that, here's a counterpoint: I can list multiple different situations where there can be problems related to L2 or the database or the backplane and so on, and FusionCache cannot do anything about it. For example:
In cases like these FusionCache can't do anything, because it cannot possibly know that you are now using only L1 but in a different environment you'll use L1+L2 or L1+L2+backplane. It also cannot know if the serializer you'll be maybe using in another environment can or cannot handle a certain type (eg: Protobuf-net does natively support In my opinion (but let me knowwhat you think) this is what tests are for: for example I'd create a test that uses a
I agree about this, and getting back to the idea of helping discovering such issues early, do you have an idea for how to catch such issues? I would gladly do something, if possible at all. Thanks. |
@jodydonetti Thanks Jody. Yes, you have fair points. There are a lot of cases where FusionCache can't do anything, but I do think this is a case where a small quality of life feature can help. I think there could be two ways to improve DX, I'd like to know what you think of them. Both options should only be enabled in debug / development mode and not for production. "Production mode" should be as is. First option: add an option to add serialization support in L1, which would be enabled in debug / development mode only. In both cases the developer (consumer) of the NuGet package should be able to configure this explicitly. FusionCache should not contain any logic to determine whether it's in debug / development mode. |
You already can, by using AutoClone.
You already can, by using Let me know what you think. |
Wow, really? I will respond hopefully Monday, I'll look into this. Thanks! |
Describe the bug
The L1 cache does not use any serialization, L2 does. Because of abstract types (or interfaces) the values retrieved from L2 are broken.
To Reproduce
Here's a MRE (Minimal Reproducible Example) of the issue:
docker run -p 6379:6379 --rm -it redis
Expected behavior
I would expect the crash would also happen on line 17. Because there is no serialization in L1, everything is OK. I acknowledge adding serialization in L1 would cause additional overhead, so I'm not sure if that's the way to go.
Versions
I've encountered this issue on:
Additional context
Adding
TypeNameHandling.Objects
to the FusionCacheNewtonsoftJsonSerializer settings, fixes this specific case (but not the overall issue).The text was updated successfully, but these errors were encountered: