MongoFramework icon indicating copy to clipboard operation
MongoFramework copied to clipboard

Concurrency Support

Open JohnCampionJr opened this issue 3 years ago • 4 comments

Not urgent on my part, but at some point I'd like to add concurrency support similar to EntityFramework.

  • [ ] Support ConcurrencyCheck Attribute for entity fields
  • [ ] Add List of Concurrency fields to entity definition
  • [ ] Add MappingProcessor to look for these fields and update entity definition
  • [ ] Update write and delete commands to check concurrency fields in the definition and add to MongoDB driver filter
  • [ ] Throw our own new MongoDbUpdate[Delete]ConcurrencyException (can't reuse EF's exceptions as they are built-in)

Feel free to assign this to me if you want....

JohnCampionJr avatar Oct 10 '20 02:10 JohnCampionJr

Interesting idea. My experience with Entity Framework is actually less than my experience with LINQ-to-SQL but it had a similar function where if a row was changed, it would throw an exception - I had to deal with a lot of "Row not found or changed" exceptions in one legacy application I worked on quite a bit! Personally I've never really liked the concept, preferring a last-write-wins approach.

For implementing the change though, what you described sounds generally good. The update and delete commands could easily grab the entity definition, traverse the properties and find ones marked for concurrency. Creating the filter for the write/delete with the additional properties shouldn't be a problem. This alone would guarantee that changes only occur by a single write at a time for a particular document however problems start when we want to throw an exception on failure.

While bulk write does return information about the result, it seems limited to how we could determine whether a concurrency failure occurs. Technically MongoDB can return failed writes however the MongoDB Driver seems to only return processed write models. Now I couldn't find information with some quick searching whether "failed writes" or "processed write models" actually factor whether an update/delete didn't write because a match couldn't be found. A write model could be considered "processed" even if it didn't match any documents. Similarly a "failed write" might not include no-matching records on a filter as a real failure compared to say conflicting a unique property/index.

For arguments sake though, let's say this "processed write models" only contain writes that affect existing records. We'd need to then compare against the list of write models we tried to save to find out which ones didn't save (compare the two enumerables to find only the differences). From there, we'd need to determine information from the write model to see whether it is compatible with the concurrency checking logic. By that I mean, we have RemoveEntityCommand and RemoveEntityByIdCommand both generate a DeleteOneModel but only RemoveEntityCommand can actually have concurrency support. We'd likely need to re-interpret the generated filter and see what properties we are filtering, seeing if any of those properties are marked for concurrency checking.

After we get past that there is one more hurdle with how we can actually trigger the exceptions. We call BulkWrite multiple times because it is a generic method, each call is for a different entity type/DBSet. As we don't have transaction support, we'd need to collate all of these errors while processing all the BulkWrite operations otherwise we'd only have a partial write to the DB for entities the user could be entirely unaware of. The collation of errors would then need to be actioned by throwing an exception.

So it is maybe possible assuming "processed write models" only includes updates/deletes that had matching records however even with that, there are likely to be a number of hairy bits in the code.

Turnerj avatar Oct 10 '20 07:10 Turnerj

One minor aside in case you are curious, my comment about calling BulkWrite multiple times because it is a generic method, while true, is effectively a limitation of how the Driver along with MongoDB works. The WriteModel class is a generic abstract class, requiring you specify the entity type as the generic parameter. This is required because MongoDB does bulk writes per collection, not per database. Internally I'm guessing it re-converts the generic parameter to a BsonClassMap just so it gets the collection name.

When I get to the lower level functionality replacement of the Driver (which is a while from now), I'll be able to avoid both the reflection overhead in MongoFramework but also wrap the strangeness better for bulk writing so it can be treated as one operation. Internally bulk writes will still have to be per collection however I can use the entity definition rather than a generic parameter to find that. May also help with support transactions and better profiling across bulk writes.

Turnerj avatar Oct 10 '20 07:10 Turnerj

Generally, I'm fine with last-in wins and there are definitely some hurdles to overcome to implement this which is why I'm not in a big hurry to do it either LOL

Based on what you said, I'm thinking this may end up needing to wait until either the Driver replacement or transactions support.

Short term, only other thing I can think of would be to sort of bolt this one as a separate case, maybe requiring Concurrency changes to be saved one at a time? It's outside the EF norms, but then again, we are using MongoDb. Something like a SaveConcurrentChanges<ConcurrentEntity>(entity).

Feels kind of messy and inconsistent and I don't need this any time soon, so I'm thinking this one goes on hold.

JohnCampionJr avatar Oct 11 '20 14:10 JohnCampionJr

Yeah - I'll leave the issue open for now as it is still a good idea but yeah, there are definitely some blockers that make doing this a lot harder than it is worth at the moment.

It's outside the EF norms, but then again, we are using MongoDb.

😅 Yeah, it is one of those things where we pick which battles we want to deal with for what we get in return.

Turnerj avatar Oct 11 '20 14:10 Turnerj