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

[FEATURE] ♊ Auto-Clone #279

Closed
jodydonetti opened this issue Aug 4, 2024 · 1 comment
Closed

[FEATURE] ♊ Auto-Clone #279

jodydonetti opened this issue Aug 4, 2024 · 1 comment
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@jodydonetti
Copy link
Collaborator

jodydonetti commented Aug 4, 2024

Preface

In general is never a good idea to mutate data retrieved from the cache, it should always be considered immutable/readonly.

To see why, the typical example of such behaviour is an update flow: get something, change it, and then save it back to the database, seemingly easy peasy.
The problem in this case is that a cache is a cache, meaning the value may not be the very last version from the database, even by just 1s or less: since we are updating it, we want to have the very last version before changing it and saving it back, otherwise we would loose some changes made after the last time cached it.

Although optimistic concurrency, with a variation of etags/last modified, surely help and helps avoid critical issues like loosing some changes, by enlarging the temporal window where something may be stale we are increasing the chance of optimistic concurrency errors.

Therefore in these scenarios (like the update flow mentioned above) the best practice is to bypass the cache entirely and just get it from the database while using the cache for the remaining reads which, in the vast majority of real-world use cases, comprise 99%+ of the overall reads (typically systems are more read-heavy than write-heavy), so we can keep the incredible perf boost with caching.

Having said that, keep reading...

Problem

It came up multiple times a desire to be able to get something from the cache and safely change it, since this may not necessarily be related to an update flow or because, simply, users may have a particular scenario in mind, and ideally they should be abe to just do that in an easy (and optimized!) way.

Currently a common solution is to create a wrapper implementation of IMemoryCache which is passed to FusionCache and would act on an underlying IMemoryCache instance by serialize + deserialize an entry at each get operation.

This technically works, but it has a couple of issues:

  • it does not get the same "it just works" vibe I always pushed for in FusionCache
  • it requires some extra coding (the wrapper class)
  • it requries some extra setup
  • since it's done at the entire cache level, it lacks granular control over which cache entries that is needed, it's an all-or-nothing approach
  • it deserializes for every cache get, which is the whole point, but it also serialize every time and from performance perspective this is not so great: deserializing is what guarantees us cloning, while the serialization to byte[] can happen only once

Solution

Since I collected enough evidence that this is something needed by the community (even though probably not by a huge amount of people), I decided to finally tackle this with a new option called Auto-Cloning that:

  • just works, out of the box
  • is easy to use
  • does not require extra coding/setup (it's just a new option)
  • uses existing code infrastructure (eg: IFusionCacheSerializer)
  • has granular control on a per-entry basis
  • is performant (at least as much as possible)

That's cool, but how?

Well FusionCache always had the ability to specify a serializer (type IFusionCacheSerializer) to be able to work with the distributed cache: it will use the same serializer to (de)serialize values, easy peasy.

To avoid being forced to also specify A new method SetupSerializer(serializer) is being created, while also being able to do the same via dependency injection, as always.
The option ReThrowOriginalExceptions will also be respected.

There will be a new entry option, namely bool EnableAutoClone that will trigger the mechanism (default: false).
Regarding performance: instead of serializing + deserializing at every get call (which would be a waste), FusionCache will keep track of an internal buffer on each memory entry and, only if and when it will be requried, it will serialize it (once), so that the binary payload will be available to deserialize at every get call.

Extra care will be put into avoiding any synchronization/double-serialization issue related to multithreading/high-load scenario.

Finally, since the feature create the clear expectation to be able to get something from the cache and freely modify it without repercussions, an exception will be thrown in these cases:

  • if the feature is enabled and there's no serializer, it will be thrown an InvalidOperationException
  • if the serializer fails to (de)serialize, it will be thrown either the specific exception (by the serializer being used) or a FusionCacheSerializationException, depending on the ReThrowOriginalExceptions option

Of course DefaultEntryOptions are always at our disposal to do the usual default + granular change flow, so we can both enable it granulary per-call or just once in the DefaultEntryOptions and forget about it (but remember: auto-cloning has a cost, so use it with care).

Alternatives

The wrapper class mentioned above, with all the issues it does have.

Additional context

The upcoming HybridCache from Microsoft will seemingly have a similar feature, meaning it will clone cached values before returning them, so feature-wise this would be aligned with that.

@jodydonetti jodydonetti added the enhancement New feature or request label Aug 4, 2024
@jodydonetti jodydonetti added this to the v1.3.0 milestone Aug 4, 2024
@jodydonetti jodydonetti self-assigned this Aug 4, 2024
@jodydonetti jodydonetti pinned this issue Aug 4, 2024
@jodydonetti jodydonetti changed the title [FEATURE] ♊ Auto Cloning [FEATURE] ♊ Auto-Cloning Aug 4, 2024
@jodydonetti jodydonetti changed the title [FEATURE] ♊ Auto-Cloning [FEATURE] ♊ Auto-Clone Aug 4, 2024
@jodydonetti
Copy link
Collaborator Author

Hi all, v1.3.0 is out 🥳

@jodydonetti jodydonetti unpinned this issue Aug 11, 2024
oskogstad added a commit to digdir/dialogporten that referenced this issue Dec 2, 2024
<!--- Provide a general summary of your changes in the Title above -->

## Description

<!--- Describe your changes in detail -->
Enables
[AutoClone](ZiggyCreatures/FusionCache#279)
feature in FusionCache
## Related Issue(s)

- #1226 

## Verification

- [x] **Your** code builds clean without any errors or warnings
- [x] Manual testing done (required)
- [ ] Relevant automated test added (if you find this hard, leave it and
we'll help out)

## Documentation

- [ ] Documentation is updated (either in `docs`-directory, Altinnpedia
or a separate linked PR in
[altinn-studio-docs.](https://github.com/Altinn/altinn-studio-docs), if
applicable)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Introduced a new property, `EnableAutoClone`, in the caching
configuration to enhance cache management.
- **Bug Fixes**
- Activated the assignment of an empty list to the `SubParties`
property, ensuring no sub-parties are included in the authorized parties
result.
- **Style**
- Minor formatting adjustments made for improved readability in the
caching configuration.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
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

1 participant