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

IVolatileDocumentManager fails to return updated document during same request #16014

Open
deanmarcussen opened this issue May 8, 2024 · 2 comments
Labels
Milestone

Comments

@deanmarcussen
Copy link
Member

deanmarcussen commented May 8, 2024

When using the IVolatileDocumentManager to cache documents, updates to said document are commited at the end of the scope.

What can occur, is that after updating an item, you also want to retrieve it in the same request, to perform some other operation.

This can fail, unless the method used to retrieve the item is GetOrCreateMutableAsync

the UpdateAsync method, places the document in the ShellScope, and if you later make another update to said document, this updated version is retrieved.

However, if you retrieve said document with GetOrCreateImmutableAsync, the ShellScope is not checkedl, so you get the old document back.

Obviously the fix for this it to call GetOrCreateMutableAsync when you require it.
This is actually very hard, because it is hard to know when to call it, as often these are quite disconnected handlers, that are working in coordination.

One solution is to prefix calls with a check on the shell scope to see of the relevant document is there... but that's just a bit of a hack (and quite frankly involves knowing way too much about the IVolatileDocumentManager)

I think it probably needs to check for a volatile cached object in GetOrCreateImmutableAsync as well, but you may end up with a document that is at that point mutable. (noone will know, but maybe it will also cause odd behaviour)

@sebastienros sebastienros added this to the 2.x milestone May 9, 2024
@sebastienros
Copy link
Member

I'd like to suggest to remove the whole DocumentManager concept, but also I tried once and then realized why it was not possible without losing something else. Don't recall what, should try again.

Thinking about delegating the distributed consistency issue to an underlying implementation (see hybrid cache in aspnetcore 9). If anyone has pointers to the DocumentManager goals I'd like to refresh my memory.

@deanmarcussen
Copy link
Member Author

The DocumentManager concept originally rose because we wanted to memory cache individual YesSql documents, e.g. TemplatesDocument is a good simple example.

It then grew to support a distributing caching arrangment.

And it became more complex, as we threw things like Autoroute at it, and more complex documents, than the simple template example.

For me, the concept is generally good, we need / want a way of caching those individual documents, like templates, so they are rendering from memory cache pre request, not retrieving from the database per request.

However, where it significantly falls over / has issues / creates unneeded / unwanted complexity, is with this idea that got introduced at some point (to fix an unanticipated problem), of retrieving a mutable or immutable document.
It introduces to much complexity about consumer intent, and means this ide of GetMutable and GetImmutable polutes the consuming code.

I suggest to JT after c# records came out, that we look at using them, to create more immutable documents, so we no longer have this pollution, but we never went anywhere with investigating it.
Ultimately because collections on records are not automatically immutable, it probably doesn't solve the issue, but it still feels like a concept worth exploring,

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

No branches or pull requests

2 participants