search

Eventuous / eventuous

Event Sourcing library for .NET
https://eventuous.dev
Apache License 2.0
447 stars 71 forks source link

Support IMemoryCache in AggregateStore #220

Closed ugumba closed 1 year ago

ugumba commented 1 year ago

This builds on #218 and #219, and adds support for IMemoryCache in AggregateStore, using StreamName as key and Aggregate.Snapshot as value.

Edit: also added to FunctionalCommandService. I'm not using this myself, but it was fairly straightforward. In general, the repo needs aligning on int vs long/ulong for versions/positions.

alexeyzimarev commented 1 year ago

My thought was to bring the cache to the lower level (event reader) because it will cover both OO and functional.

ugumba commented 1 year ago

@alexeyzimarev, that looks a bit awkward to me, as IEventReader knows only about events, not the state (or aggregate). Would you cache the events instead of the state?

Also, I assume you mean the event store, as the cache should be updated with the new events/state, otherwise the next command would have to redundantly re-apply the most recent event(s).

AggregateStore ("OO") and FunctionalCommandService/FoldedEventStream ("functional") are the two current structures which instantiate state, fold in past events and store new events. Without a larger refactoring, I'm not sure how to cache closer to the event reader.

(Frankly, I don't quite understand the motivation for FunctionalCommandService - you've basically reimplemented CommandService/Aggregate as FunctionalCommandService/FoldedStream using different names and a slightly changed syntax...)

In general, I'd assume that we should avoid referencing the full event stream as much as possible - or we're guaranteed to create a scalability problem. (That's the reason for #218, but I think it is equally undesirable in FoldedEventSteam.)

Edit: fixed typo

alexeyzimarev commented 1 year ago

I am not sure what you mean by FunctionalCommandStore as it doesn't exist.

alexeyzimarev commented 1 year ago

Yeah, you are right about the reader as it only knows about events. My, maybe irrelevant, concern is that the aggregate is nothing more than a set of functions. The persisted part of it is the state object. I think it is ok to cache aggregates only for now, I will figure out the rest later.

ugumba commented 1 year ago

I am not sure what you mean by FunctionalCommandStore as it doesn't exist.

Sorry about the typo, I've corrected to FunctionalCommandService.

I think it is ok to cache aggregates only for now, I will figure out the rest later.

The proposed use of IMemoryCache caches state and version (in a Snapshot) - the original events are not included, and could make caching quite expensive (that's why I took them out).

bartelink commented 1 year ago

Definitely best to cache state+version - this might be relevant https://github.com/jet/equinox/blob/master/src/Equinox.Core/Caching.fs https://github.com/jet/equinox/blob/master/src/Equinox.Core/Cache.fs

alexeyzimarev commented 1 year ago

Something tells me that caching and snapshotting are very close conceptually. Yes, snapshotting implies serialisation, but so does caching in a distributed store like Redis.

I would also attempt to avoid changing the existing signatures in favour of composition. I realise it might not be possible with the current implementation, so some changes might be required in different abstractions.

ugumba commented 1 year ago

I've had to move on - my fork will remain if anyone wants to consider picking this up again, or would like to discuss.