[RFC] remove $identityMap from AggregateRepository
I would like to discuss to remove the caching of aggregates via identityMap. The main problem with this are long running processes like projections. If a projection have to load the aggregate, the wrong state can be loaded. The user has also to ensure not to run in memory exhausting. This is error prone too.
Assume that PHP process PhpA is a long running projection which have to load the aggregate because it replaces the whole read model entry for this aggregate and not only the data from the event. The PHP process PhpB is our write process.
PhpB -> event1
PhpA -> event1
PhpB -> event2
PhpA -> event2 (but it gets the state of event1 via repository->get() because of identity map)
I'm okay with removing, what do you think @codeliner ?
Can second removal. With command handlers in background jobs an AR effectively becomes bound to a specific process (unless the map is cleared).
- Process A loads AR, pushes new version 2
- Process B loads AR, pushes new version 3
- Process A has AR in cache, tries to push new version, ConcurrencyException but old version is kept in map
- Command is retried, fails until it reaches process B
We "solved" this by wrapping an repositories save function to just catch concurrency exceptions, clear the cache, and rethrow the exception for all our repos. If caching is required this should be delegated to a strategy or facade imho.
Maybe we should do this in current version too, if accepted.
At least some flag / option on the repo to disable it would be helpful indeed :) Leave it on by default for BC.
@sandrokeil @fritz-gerneth
At least some flag / option on the repo to disable it would be helpful indeed :) Leave it on by default for BC.
This can be done immediately, as there is no BC, just submit a PR incl. tests and I will merge and release this change.
yeah, we can remove identity map with next major release. In earlier versions we used it to keep the illusion that an aggregate repository can be an in-memory collection (no $repo->save($ar) call needed), but current repository interface has a save method and a unit of work is not required/no longer used.
Additional flag to deactivate identity map in current version sounds good, too.