U4-6147 - Update DB Transaction locks to be correct

Created by Shannon Deminick 16 Jan 2015, 07:25:26 Updated by Shannon Deminick 03 May 2016, 20:20:38

Subtask of: U4-5482

Moving to repeatable read locks

Comments

Stephan 03 May 2016, 14:03:20

PR: https://github.com/umbraco/Umbraco-CMS/pull/1249

What this does

Cleanup IoC for repositories. Cleanup the repository factory, units of work, services.

Ensure that every Umbraco transaction is now a RepeatableRead transaction.

Repositories are ''not'' IDisposable anymore. A repository manages entities in the database but is not responsible for the unit of work / transaction aspect of things.

An IUnitOfWork ''is'' IDisposable, now. A unit of work ''does'' manage the transaction aspect of what is happening. And so the proper pattern in services becomes:

using (var uow = UowProvider.CreateUnitOfWork()) { var repo = uow.CreateRepository(); repo.AddOrUpdate(myThing); uow.Complete(); }

When the unit of work is completed, its queue is flushed (ie operations are actually executed against the database), and it is marked as "complete". When the unit of work is ''disposed'', if it is complete then the underlying transaction is committed, else it is rolled back.

Refactor the "object database-level locking" which was used at the moment for server registration only and consist in locking a row in umbracoNode. Got rid of LockedRepository and LockingRepository and going for a more elegant/lightweight solution. Now can do things along:

using (var uow = UowProvider.CreateUnitOfWork()) { var repo = uow.CreateRepository(); repo.ReadLockRegistrations(); // acquire database-level lock // do what we have to do... uow.Complete(); } // the lock is released at the end of the transaction

Review

Should be a code review, mostly. Everything should just work as normal. Also, all tests should pass.

Discuss

Should fix server registration repository (see U4-7046) now that all transactions are RepeatableRead. => U4-8397

Not implementing service-level unit of work. => U4-8398

Need to deal with events. Currently, events such as "Saved" trigger before the unit of work / transaction is actually commited, meaning that if it is rolled back for some reason, it's too late and the event have already been processed. => U4-8399

Need to deal with repository's cache. At the moment, as soon as eg something is persisted, the repository's cache is updated, meaning that if the transaction is rolled back for some reason the cache is corrupted. Plus, see discussion in U4-7046, we need means to disable repository's caches in some situation as these caches are inherently not safe. => U4-8400


Shannon Deminick 03 May 2016, 20:17:41

the code all looks good so far, i didn't go over it with a fine-toothed comb but had a look at all files in each revision and I think it all makes sense.

Regarding the service level (wrapping) transations and event management, I think we can handle both of these things by extending the unit of work. If we create a transient type of ServiceContext, the ctor can accept a UOW which it then passes down to each service ctor (maybe as an overload). The UOW can also track all events to raise in a queue so that when it's successfully completed, it will raise all events. This would then also work for Deploy because we'd be able to postpone events until the full thing is done.

I'm still running tests but i think it's all good.


Priority: Normal

Type: Task

State: Fixed

Assignee:

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions:

Due in version: 8.0.0

Sprint: Sprint 14

Story Points:

Cycle: