Optimistic concurrency control - will we need it?

Coordinator
Nov 18, 2011 at 9:12 AM
Edited Nov 18, 2011 at 9:12 AM

Our requirements say that we will have to support a maximum of 5 concurrent users. I guess its not very likely that more than one user is working on the same sponsor or child, so there shouldn't be any concurrency issues in the first place. But still i want to make a case for concurreny control: We don't know how the sponsorship agency will evolve. Maybe in a couple of years there will be working 15 people against the same data storage. I think it's more than likely that problems will start to occur in this scenario.

Because of this I have implemented the necessary changes to support optimistic concurrency control.
As soon as I get the green light I can check them in.

Of course a feature like that doesn't come without costs, but in my opinion the necessary changes to our commands and read models are minimal.

What do you all think?

Coordinator
Nov 18, 2011 at 9:36 AM

You should post a small sample which changes are necessary. I guess you will transport the version number.

Add concurrency: it's only important when sending the same commands for an aggregate. F.e. sending the command 'changeSponsorEmailAdress'

Coordinator
Nov 18, 2011 at 9:58 AM

I think that unless it makes everything much more complicated, we should strive to handle concurrency from the start. Something like this is always much more difficulat to add afterwards.

Coordinator
Nov 18, 2011 at 10:27 AM

Yes, I transport the version number of the aggregate.We would only have to change the commands and the read models.

These are the necessary changes with the example use case "Change a childs name":

1) A ChildListDto is loaded, we create a new ChangeChildName command and send it over the bus to the command handler

var readRepository = ObjectFactory.GetInstance<IReadRepository>(); 
var bus = ObjectFactory.GetInstance<IBus>(); // command bus

var child = readRepository.GetAll<ChildListDto>().First();
var cmd = new ChangeChildName(child.AggregateRootId, child.Version, "Peter", "Pan");
bus.Send(cmd);



2) CommandHandler.cs

public void Handle(ChangeChildName command)
{
    //get child aggregate from the eventstore
    Child child = _repository.GetById<Child>(command.ChildId, command.Version);
    
    //execute domain logic
    child.ChangeName(command.FirstName, command.LastName);

    //save new raised events of aggregate into eventstore     
    _repository.Save(child, Guid.NewGuid());
}



3) ChildListView.cs

public void Handle(ChildNameChanged domainEvent)
{
    using (IDocumentSession session = _documentStore.OpenSession())
    {
        ChildListDto dto = session.Load<ChildListDto>(Dto.GetDtoIdOf<ChildListDto>(childId));
        
        dto.Name = domainEvent.FirstName + " " + domainEvent.LastName;
        
        SaveChanges(session, dto, domainEvent);
    }
}

private static void SaveChanges(IDocumentSession session, ChildListDto dto, IEvent domainEvent)
{
    // Getting the event version out of the event headers
    dto.Version = Int32.Parse(domainEvent.GetHeaderValue("EventVersion"));

    session.SaveChanges();
}

 

Bold text marks the changes. As you can see I didn't have to change the domain class (Child.cs) or the events.

There are some changes to the infrastructure, but those are not important when implementing new use cases.

Coordinator
Nov 18, 2011 at 10:39 AM

For the commands: you only have to change the code generator once and it will be added to all commands automatically.

Coordinator
Nov 18, 2011 at 12:40 PM

That is true, but I think it is important to set the version in the constructor of the command.

So something like that ...

let firstName = string FirstName;
let lastName = string LastName;
let lastName = string LastName;

ChangeChildName? (childId, firstName, lastName)

... would result in this:

public sealed class ChangeChildName : ICommand
{
    public Guid ChildId { get; private set; }
    public string FirstName { get; private set; }
    public string LastName { get; private set; }
    public int Version { get; private set; }
        
    public ChangeChildName () {}
    public ChangeChildName (Guid childId, string firstName, string lastName, int version)
    {
        ChildId = childId;
        FirstName = firstName;
        LastName = lastName;
        Version = version;
    }
}

The downside of this approach is that there would always be version properties, even if you don't need them (e.g. CreateChild has always version 0).

Coordinator
Nov 18, 2011 at 1:31 PM

Caveat: I haven't had the time to fully dive into the source code yet, so maybe this is obvious, but: how would be perform the actual concurrency control? I.e., what code checks the actual version against the expected version, and what code increases the current version?

I'll assume it would work as follows:

  • In the UI, when displaying a child, we get the version of the child currently being displayed from the read side.
  • When constructing the command, we pass in exactly that version (your step 1).
  • When handling the command, the version is passed to the repository (your step 2). The reads the child and checks whether it still has the correct version (i.e., the version of the last raised event is the same as the version passed in). If not, it throws a ConcurrencyViolationException. Otherwise the domain logic is executed and raises an event that includes an increased version. This is saved in the event store.
  • The read-side event handler now constructs a view with the increased version. The next time, when displaying a child, that version is used.

Can you comment on this, is my understanding correct?

If yes, there is one issue: Within my third step (reading the child from via repository, checking its current version, then publishing an event with the new version), there is a window for a race condition, unless the command handler is transactional. How is this handled?

And then, of course, we'll need to define whether all commands really need to perform version checks. And whether commands using more than one aggregate need to perform version checking on both.

Coordinator
Nov 18, 2011 at 2:30 PM

Your comments are right. Reading from the eventstore, bringing the aggregate to the current state, executing the business logic and storing the new published events into the eventstore is transactional.

-> writes on the write side are transactional. The used 'Event Store' library handles that for us.

You will use sagas for commands that has effects on more than one aggregate but I don't see any case in the current requirement set.

Coordinator
Nov 19, 2011 at 2:50 PM

Yes I think sagas won't be necessary.

@Fabian: I'm not sure if there really exist commands that don't need version checks except commands that create new aggregates.
As every command is bound to change state, I think it also needs to know for which version of the aggregate it is targeted.

So I guess if nobody has a strong argument against optimistic concurrency control its ok if I commit my changes.

Coordinator
Nov 19, 2011 at 9:35 PM

@Sebastian: Consider the use cases of adding payments. When two people enter payments to be tracked for the same aggregate (SponsorPayments?) at the same time, we probably shouldn't check for concurrency problems. I.e., it's okay when the command is executed on a different version of the aggregate than the user saw when entering the payment.

Coordinator
Nov 20, 2011 at 8:43 AM

Ah, you are right, didn't think of that!

Coordinator
Nov 20, 2011 at 5:23 PM

We should probably determine for every command: does the execution of this command depend on the previous state of the aggregate as displayed to the user? If yes, the command should perform a version check; if no, it shouldn't.