aggregateLock can't 100% prevent concurrent update

[emmkong]

Hi @adrai

I testing the aggregate lock in our end to end integration tests and found that (maybe I'm using it wrongly) it not able to prevent concurrency happening in a multiple processes environment. (example AWS Lambda) perfectly.

Scenario:

1. 3 or more commands (apply to the same aggregate) triggered the domain at the same time.
2. Each commands handled by different Lambda instance. (sharing same aggregate lock DB)
3. Each Instance locked the aggregate simultaneously and loaded the same version of history events (3 locks with the same aggregateId, different workId generated )
4. Instance 1 attempt to commit but failed due to the aggregateCheck, Then, all locks resolved and the command scheduled to be retry.
5. Instance 2 committed successfully as the lock being resolved by instance 1.
6. Instance 3 (with outdated history events) also committed successfully after the lock being resolved by instance 2.

I found the following line of code cause this scenario:

cqrs-domain/lib/defaultCommandHandler.js ln 663

if (workerIds.length === 0 || (workerIds.length === 1 && workerIds[0] === self.id)) {
        return callback(null);
}

I suggest we could tighten up the condition like following:

if (workerIds.length === 1 && workerIds[0] === self.id) {
        return callback(null);
}

and we should also allow a bigger retryOnConcurrencyTimeout value so that the retry not coming back at the same time again. Any thought?

[adrai]

Yes, you are right. This was coming from an older version that handled this a bit differently... Do you want to create a PR?

[emmkong]

@adrai Thanks for your reply. Yes, I will submit a PR after my local end to end testing. Do you agree with my suggestion or you have any better solution for this problem?

[adrai]

your suggestion is perfect