Closed alexeyzimarev closed 7 years ago
I'd like to see what the this would look like. Persistence is a pretty small class so maybe knock up a demo to discuss?
Something I was wondering - would you ordinarily save a snapshot in the same stream as the events, or use a different stream?
@jrouaix weren't you working on an event store provider too?
I will check if I can actually make a change. I spent some time on the provider itself, got stuck after realising that snapshot index would require the event index, which I have not found where to store yet - either I need a wrapping class or store the version in the metadata.
Concerning snapshot storage - I would keep it in a separate stream. This would mean that to read the last snapshot I just need to read one event from the end of the stream, which is very fast. From there I would know the snapshot version and can read events from that version up to the end of the event stream.
As you can find in my code, I offer a possibility to specify custom stream names with default strategy adding snapshot-
before the actor name, which normally would be the event stream name.
I've just remembered one of the first things i did with Proto.Actor was try and write an event store client. man, i'd forgotten all about that. it is very out of date now (288 commits behind AsynkronIT:dev.):
And this:
public async Task DeleteSnapshotsAsync(string actorName, long fromIndex)
{
throw new NotSupportedException("EventStore is an immutable database that does not support deletes");
}
:)
Yeah, this is one way of doing that. I am not sure why deleting some snapshot would be needed. However, I can see that at the end of actor's lifecycle we might need to remove all traces of it.
I am not sure how it ideologically fits to the virtual actor pattern, since as I understand it, virtual actors always "exist". But my understanding of the Proto.actor is lacking, this is why I actually created this issue and not submitted a PR.
Having -snapshots
as a suffix might be preferred to make a category of it...
And I see you stored index in the metadata, like I mentioned too. The code is very close to what I am writing. I am not sure that is so much out of date since event read/write API has not changed much in ES for a while.
Persistence isn't hooked in to virtual actors at all at the moment. @rogeralsing has raised this recently. The other thing about the persistence stuff is that it isn't actually integrated with Proto.Actor as such - it is really just a simple abstraction over event sourcing that we've knocked together, but you could equally just use something like NEventStore - there's nothing special about the relationship between the Persistence class and Proto.Actor (the Persistence project has no references to any other Proto project so could easily be a standalone thing). This might change in the future if it's hooked into virtual actors / grains, but for now, it is standalone.
I wrote a little about the persistence plugin here http://proto.actor/blog/2017/06/24/money-transfer-saga.html#3 which might give a better idea of how it is used within proto actor.
@tomliversidge yes i was working on an event store (geteventstore.com) persistence when you onboarded the project ... then I had no time to catchup the fast pace of this project. I just dumped my super old fork yesterday.
I'll probably have implement it anyway if we integrate proto.actor. Plus now I have to implement a Kafka EventStore persistence as well, because we are integrating Kafka (GES didn't take enough load).
I don't think implementing the ES persistence inside this repo will make sense until they have a working netstandard client. So far there is only one 4.0 alpha client is published and not updated for three months. Of course I can add a project with one framework (4.6.1) support but this means there will be no netstandard package out of it.
For now, the open question for me are:
1) Does it make sense to remove an actor state after it is completed/done with the work? If yes - deletion must work.
2) If the previous question answers yes, then having server-side version is a must, so the Persistence
class needs to be changed accordingly
When this is clarified, I can finish the provider.
@alexeyzimarev you are right, one nice way to implement snapshots in GES is to have to them on another dedicated stream (in my company we chose to prefix, in order to have a category for snapshots). You can also configure the snapshot stream do have only one last "event" available by updating the stream metadata to have maxCount: 1. This way GES will be able to scavenge the previous snapshots, and you, can just pick the only data available on the stream to get you snapshot.
@jrouaix yeah, I know all that, we have it in production for quite a while. So I just need to clarify the technical design questions in order to implement properly.
@alexeyzimarev @tomliversidge About indexing the events, it's a tricky one. With GES we have a guaranty that the EventStore indexes are the same than the aggregate (but no more in case of soft deletion). But I had a hard time to guaranty that with Kafka (this one can double write, or even change the ordering of batch of events, so I had to implement some sequence number thing in our solution).
Does it make sense to remove an actor state after it is completed/done with the work? If yes - deletion must work.
IMO, if the actor will never ever be spawned, I could want to delete it's state ... but it could be a feature provided the store itself (retention time in Kafka or GES).
If the previous question answers yes, then having server-side version is a must, so the Persistence class needs to be changed accordingly.
Agree when server is GES, not with kafka without some overrides. Sure that eventstore.GetEventsAsync could return some more complex informations such as "StoreIndex", and perhaps the IEventStore could expose a property to know if the underlying technology should be master of the "Index".
Seems to be solved now :)
Currently, the event index, which is used to read events and create/read snapshots, is driven by the Proto.actor. This creates issues when trying to support immutable event stores (like geteventstore.com) because there the event index is driven by the store. Since it is impossible to remove anything there, the only way to support
DeleteEventsAsync
andDeleteSpanshotsAsync
is to soft-delete streams, new events will get version not starting from zero. With current model, when event index starts from zero and event version returned from the store cannot be used, makes it impossible to support immutable event stores at least with snapshotting, since querying by index will only be possible by traversing all events, so snapshotting makes no sense there.