Meeting feedback about ISession

[jtkech]

@sebastienros @MikeAlhayek

Just saw one of the last meeting where you talked about reloading an item (after a long process) before saving.

From memory IContentManagerSession only prevent to do a query to YesSql, so clearing it may not be sufficient because the YesSql ISession has its own cache.

So one solution would be to use ShellScope.UsingChildScopeAsync() to be under a new ISession, but the simplest way (if it fixes your issue) would be to use ISession.Detach(), we use it here.

OrchardCore/src/OrchardCore/OrchardCore.Data.YesSql/Documents/DocumentStore.cs

Lines 73 to 76 in 72d717f

	if (!_committed)
	{
	_session.Detach(document);
	}

[MikeAlhayek]

@jtkech thanks for that info. What I don't understand is that I am using Session directly without the contentManager. I simply query the content items, then alter some parts (the process may take 20 seconds before the alter take place). Then, I save it directly to the session. I think the background task will create a new session every time it runs since I am retrieving an ISeasion instance from the IoC container. I will send you the code I was referring to on Element/gitter. Lets me know your thoughts

[jtkech]

I think the background task will create a new session every time it runs

In fact it uses a new shell scope and your bg task just have to inject ISession if it is a scoped service, or resolve it from the passed service provider. Then it depends on your code, the problem being to only load the item before the long process and then save, this under the same session.

So my suggestion after the long process would be to call ISession.Detach(item), then load/query again the item and save it. Notice that the whole is still not atomic (even if transactions are atomic) but to have a better chance to not have e.g. duplicated Latest and Published.

[MikeAlhayek]

But even if I don't detach, shouldn't session not create two published version since we check for concurrency?

Or concurrency check is a check in the memory only not on the database?

[jtkech]

No, concurrency checking only check that you are not saving concurrently 2 items having the same Version field value, otherwise it throws, it's only to ensure the consistency at the YesSql document level, that's okay for document on which we don't manage different documents with different Latest and Published values but for the same contentItemId.

But if the update is not done concurrently, YesSql is not aware of our data and how we process them, this from data that we load in memory at some point, like in a temporary cache that may not be in sync in different running instances. To fully support concurrency updating at the content item level it would need to be atomic by using a lock (distributed if needed).

Note: Even if we update the same single Document without managing Published/Latest "versions" we may still want some atomicity, for example if a document need to hold an exhaustive list of elements without loosing any, we may need something like we did for the VolatileDocumentManager.

OrchardCore/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Documents/IVolatileDocumentManager.cs

Lines 12 to 16 in 72d717f

	/// <summary>
	/// Executes the provided delegate and updates the cache, the whole being done atomically and after the session is committed,
	/// this only if a lock can be acquired (default timeout to 10s), and atomically if the lock doesn't expire (default to 10s).
	/// </summary>
	Task UpdateAtomicAsync(Func<Task<TDocument>> updateAsync, Func<TDocument, Task> afterUpdateAsync = null);

[jtkech]

So here, without more atomicity, the goal is to reduce the time between the last item load and its updating