OptimisticPipelineHook throws invalid ConcurrencyExceptions with IPersistStreams.Purge

[fschmied]

We just ran into an issue in our integration tests, where IEventStream.CommitChanges throws a ConcurrencyException although there are no concurrent modifications (and indeed no commits in the database). Debugging showed that the exceptions are thrown by OptimisticPipelineHook.PreCommit - the "heads" stored by the hook did not match what was in the database.

The reason is that our tests use IPersistStreams.Purge to clear the event store, and the OptimisticPipelineHook doesn't clear its cache when that API is called.

[damianh]

Oh man... and I was just about to release it ha

[damianh]

Is a workaround to recreate the IStoreEvents viable?

[fschmied]

Yes, that is of course a workaround, although it's cumbersome (we currently use a singleton IStoreEvents in the container and changing that instance to renew after Purge in a thread-safe way will be difficult to implement). Another workaround would be to "disable" the optimistic pipeline hook by configuring the event store to enlist in ambient transactions. (There doesn't seem to be another way to prevent it from being installed ATM.)

BTW, I've just noticed that the new DeleteStream API probably suffers from the same issue - if you delete a stream, the tracked "heads" remain in the OptimisticPipelineHook if I understand the code correctly. So if you afterwards try to recreate the stream, you could get invalid ConcurrencyExceptions.

I guess the right way to fix it would be to add hook extension points for Purge and DeleteStream and adapt OptimisticPipelineHook accordingly.

[damianh]

I guess the right way to fix it would be to add hook extension points for Purge and DeleteStream and adapt OptimisticPipelineHook accordingly.

Am considering that but feels kinda leaky and I haven't come up with a cleaner way yet. I'll spike it and see what it looks like.

[damianh]

@fschmied Would appreciate a review of the PR, you've got a good eye for detail 😄

[fschmied]

@damianh I took the time today to look deeper into the code as I'm back to a real computer now, and I've found another interesting spot: it seems that the bug was originally caused not only by the fact that Purge and DeleteStream don't evict the cache, but also because one of the GetFrom overloads doesn't execute the hooks (line 35 in PipelineHooksAwarePersistanceDecorator.cs):

    public IEnumerable<ICommit> GetFrom(string bucketId, string streamId, int minRevision, int maxRevision)
    {
        return _original.GetFrom(bucketId, streamId, minRevision, maxRevision);
    }

    ...

    public IEnumerable<ICommit> GetFrom(string bucketId, DateTime start)
    {
        return ExecuteHooks(_original.GetFrom(bucketId, start));
    }

    public IEnumerable<ICommit> GetFrom(string checkpointToken)
    {
        return ExecuteHooks(_original.GetFrom(checkpointToken));
    }

    ...

    public IEnumerable<ICommit> GetFromTo(string bucketId, DateTime start, DateTime end)
    {
        return ExecuteHooks(_original.GetFromTo(bucketId, start, end));
    }

    public IEnumerable<ICommit> GetUndispatchedCommits()
    {
        return ExecuteHooks(_original.GetUndispatchedCommits());
    }

As you can see, most read operations execute the hooks and thus cause the optimistic hook cache to be refreshed, but the GetFrom(string bucketId, string streamId, int minRevision, int maxRevision) overload doesn't - which causes the stale items to remain in the cache.

I therefore believe that changing that overload to execute the hooks would also fix the cache issue, with the additional benefit of also working if another client deletes a stream.

What do you think?

[damianh]

Looks like a bug alright.

[damianh]

Created seperate issue.