NEventStore / NEventStore.Persistence.MongoDB

Mongo Persistence Engine for NEventStore
MIT License
22 stars 26 forks source link

NullReferenceException raised due to duplicate key error in Wired Tiger #45

Closed edtyl3r closed 7 years ago

edtyl3r commented 8 years ago

With MongoDB switched to wired tiger we are getting a NullReferenceException inside "PersistWithClientSideOptimisticLoop" method when there was a MongoDuplicateKeyException. In this method there is already an error handler for this error ("E11000 duplicate key error collection: EventStore.Commits index: id dup key: .....") but it was looking for an index name of "$id" here:

  if (e.Message.Contains(MongoCommitIndexes.CheckpointNumber))
                        {
                            commitDoc[MongoCommitFields.CheckpointNumber] = _getNextCheckpointNumber().LongValue;
                        }

It looks the error message is slightly different in wired tiger than previously with MMap.

I have created a pull request here: https://github.com/NEventStore/NEventStore.Persistence.MongoDB/pull/44

Also, the build is failing which I think might be due to an error in the submodule "error: no such remote ref 74fafdd719fe6abefeebc0bbfb12fa951b6bebd2 Fetched in submodule path 'dependencies/NEventStore', but it did not contain 74f afdd719fe6abefeebc0bbfb12fa951b6bebd2. Direct fetching of that commit failed." Does anyone know anything about that?

edtyl3r commented 8 years ago

We have finished testing and released the fork to production. Now running wired tiger and NEventStore under load with no issues for 24 hours.

alkampfergit commented 8 years ago

Sorry for late response.

Perfect, I'm pretty sure that your fix is ok, because we have a really similar fix on develop branch (that we are using right now). In develop branch we adopted a slightly different approach, in MongoFields.cs (https://github.com/NEventStore/NEventStore.Persistence.MongoDB/blob/develop/src/NEventStore.Persistence.MongoDB/MongoFields.cs) we inserted two constants to differentiate between the message raised with WiredTiger or MMapV1.

To avoid conflict with develop probably it is better to apply the very same strategy in develop branch and in master branch, if you agree we can bring the very same modification we did in develop branch as an hotfix of master, and close this pull request.

If anyone else has a preference (using two constants or using a single constant) please feel free to comment this pull request.

Thanks.

edtyl3r commented 8 years ago

Thanks for this. I don't think it makes much difference so I'm happy for you to close my pull request and go with the develop branch since that's going to be easier.

alkampfergit commented 8 years ago

I've created hotfix/5.3.7, feel free to check if everything is ok, then I can merge with master and publish the package.

edtyl3r commented 8 years ago

Cool - looks good to me.

edtyl3r commented 8 years ago

Is it possible for you to publish the 5.3.7 version please? Thanks, Ed

alkampfergit commented 7 years ago

It should be published now: https://www.nuget.org/packages/NEventStore.Persistence.MongoDB/