OrleansContrib/Orleans.Providers.MongoDB

Not thread/interleave save?

Xanatus opened this issue · 6 comments

Xanatus commented

It seems when 2 calls end up in WriteStateAsync() on the same grain, there is a good chance that following InconsistentStateException is thrown:

image

This can easily happen on interleaved grains or when using timers (as seen in above screenshot).

I didn't try, but I assume an easy way to reproduce would be:
Task.WhenAll(WriteStateAsync(), WriteStateAsync());

Xanatus commented

Why is this by design and why can it not be prevented?
Queuing up the queries not an option?

It used to work fine in 1.5.
If stateful grains can no longer use interleave semantics, this should be documented better. I can also see performance drops in my application due to this.

Xanatus commented

Also I think this kind of ETag checking is causing more problems than it fixes.

Consider this grain method:

[AlwaysInterleave] Task Set(int val) { State.val = val; return WriteStateAsync(); }

Now if this method is called by 2 grains at about the same time, what will happen?
The first call will succeed and the second one (who is more likely to have an up-to-date value) will fail? Seems backwards to me.
It's not preventing the racecondition from happening, it just makes it worse by discarding the newer value.

I see the point of etags preventing different applications to write to the same state, but not why the same grain can't write to same state.

Either way, how save is it, in such a case, to catch the exception and keep resubmitting the state without reloading the state first?

SebastianStehle commented

You already have an option for queuing: Do not use interleaving ;)

There might be a bug in the implementation, but it is important to follow the official semantics. If you have 2 concurrent calls making changes to a state you can have concurrency issues anyway.

Xanatus commented

Interleaving is not concurrent and there can be valid reasons to do it that way.
And its sometimes just hard to work around it. Such as that orleans timers enforces it.
And sure, knowing this restriction makes it of course possible to avoid it. Unfortunately I didn't know about it before 2.0 when etags where not a thing and refactoring this is a bitch now.

But anyway, do you think the following is save to use when not caring about consistency?

async Task WriteStateAsyncNoConsistencyCheck()
{
   try
   {
      await WriteStateAsync();
   }
   catch (InconsistentStateException)
   {
      await WriteStateAsyncNoConsistancyCheck();
   }
}
SebastianStehle commented

Thats true with concurrency. I just think it is the wrong place to discuss it. All orleans storage provider should follow the same semantics. If you want to avoid it you can just implement your own storage mechanism. Personally I also do not use Orleans Storage because I have different requirements.