primait / event_sourcing.rs

event sourcing framework in rust
48 stars 4 forks source link

[Discussion] Data conversion management #143

Open cottinisimone opened 1 year ago

cottinisimone commented 1 year ago

Esrs data conversion management

Topic

This is a discussion thread in order to decide which route should this library follow in order to manage data conversion.

State of the art

Actually esrs is implementing the weak schema technique. With this technique the library handles missing attributes or superfluous attributes gracefully, just by manually updating the event shape adding Optional fields or removing existing fields from the event. Being that this is an optimistic approach, sometimes retro-compatibility issues while loading older events from the store come out.

Techniques

Event versioning/multiple versions

In this technique multiple versions of an event type are supported throughout the application. The event structure is extended with a version number. This version number can be read by all the event listeners, and they have to contain knowledge of the different versions in order to support them. In this technique the event store remains intact as old versions are never transformed. There are no extra write operations needed to convert the store.

Implementation

A macro to put on top of an event that every time that piece of code is compiled optionally insert that new event version in a local schema registry (like a file?) and checks the event retro-compatibility. The cons with this approach is that is needed to think about a macro attribute to ignore a specific field (for example if the event store has been updated manually) or ask the user to manually fix local schema registry file.

Upcasting

Upcasting centralizes the update knowledge in an upcaster: a component that transforms an event before offering it to the application. Different than in the multiple versions technique is that the event listeners are not aware of the different versions of events. Because the upcaster changes the event the listeners only need to support the last version.

Implementation

Create a new trait Upcaster that your event must implement and having a two functions that user must implement, like upcast and actual_version. Inside of the upcast function the user should manually deserialize a json to get versions and the fields it needs in order to build latest event version?

Others

There are two other more exotic techniques to mention:

The biggest downside for those three techniques is that all those techniques perform, while reading events, updates on database, breaking the "events are immutable" dictatum.

Moreover lazy transformation and in place transformation are not reliable being that changing the event store permanently makes it mandatory to make backups.

And ofc there's the "leave it as it is" option :).

fusillicode commented 1 year ago

My "gut feeling" vote goes to... Event versioning/multiple versions 😬

P.S: https://outcrawl.com/event-versioning-rust

heartlabs commented 1 year ago

Moreover lazy transformation and in place transformation are not reliable being that changing the event store permanently makes it mandatory to make backups.

Is this because there could be a bug in the upcaster that could create invalid events (e.g. an invalid JSON) or a bug in the code that updates the db row that could corrupt the data (e.g. accidentally deletes a property)? Or would there be any other examples that would make a backup mandatory?

heartlabs commented 1 year ago

Also given that our data is really valuable - shouldn't we anyway backup our databases? Data corruption during update of an event version is just one of many ways that we could lose the data in a database

angelo-rendina-prima commented 1 year ago

What about separating between in-memory domain Events and persistence concerns? In short,

Reasons:

Having an actual separation between the two layers would make it explicit when you need to simply "upcast/version" the writable data formats into the same in-memory type, vs when you have to actually think about the business logic and just add a new domain event type. This has bitten us already in the past, where by changing in-memory event types we inadvertently broke the db.

In conclusion, at the "persistence" layer, you can actually choose any of the strategies proposed in the thread - or even just leave that as an interface/trait in the EventStore that the user has to implement themselves (granted we can provide a default implementation for the PgStore as we see fit).

cottinisimone commented 1 year ago

Is this because there could be a bug in the upcaster that could create invalid events (e.g. an invalid JSON) or a bug in the code that updates the db row that could corrupt the data (e.g. accidentally deletes a property)? Or would there be any other examples that would make a backup mandatory?

Yes, exactly. And maybe not even for a total bug. And if you remove a field because you think that you might not need it anymore and then in the future you see that that field was useful for something else (like data analysis or whatever) if you don't update the event you have the older events still contains that information, resulting in not a total loss of information. (yeah, this is not the main reason. The main reasons are bugs).

Also given that our data is really valuable - shouldn't we anyway backup our databases? Data corruption during update of an event version is just one of many ways that we could lose the data in a database

We already do it (daily?). But still, even if it's just a single day, it is a full day lost

oliverbrowneprima commented 1 year ago

Upcasting seems easier to implement from a library users perspective - they only need to implement From to convert each previous event type into the one used by the application, and be able to hand ESRS a function that consumes a &str and returns an Event - presumably by trying to de-serialize the string into the latest Event type, then the latest - 1 Event type, and so on, until either they manage to construct one or fail out.

We could write a macro to generate that code for them, something like:

let event_deserializer = event_des![serde_json::from_str::<Event>, serde_json::from_str::<EventV2>, serde_json::from_str::<EventV1>]

The alternative of asking library users to version their events seems like a lot of extra work (they'd need to write upcasting code anyway, or else have their application be able to handle every version of an event), for not much of a gain (being able to make business decisions based on event version seems like an edge case that is just as well handled by event timestamps). I think version tags have an unfortunate habit of leaking persistence-specific information and complexity into the rest of the codebase.

Trying to have a macro that auto-versions Events seems like a lot of accidental complexity - managing the list of previously seen versions, across developer machines and CI runs etc (if you check it in how do you handle merge conflicts?) will be annoying, and non-obvious to users as to how it works. It also runs against problems with backwards compatible and non-backwards-compatible changes - a user might, for example, add an Optional field, and not want to bump the event version when doing so - how do we let them? Do we auto-detect optional fields? etc.

With respect to the proposed Upcaster trait, I think it's easier if the library only asks the user to provide a fn(str) -> Result<Event, String> (or maybe it should take a serde_json::Value?), where Event is always the latest version, and then inside that fn they handle parsing the event we load from the DB by descending through their known event types, each time trying to call from_str and then their From impl that converts that particular event into the latest Event type. This way, all of the previous-event-version parsing code lives in a single function (plus those From impls), and all the rest of the code can ignore the fact that there's previously versioned events in the DB at all. As mentioned above, we could provide a macro to generate the parsing function, leaving them to only write the necessary From implementations, which is I think a fairly small ask.

For the sake of clarity, the above macro would expand into:

|data: &str| -> {
    if let Ok(e) = serde_json::from_str::<Event>(data) {
        return e;
    }
    if let Ok(e) = serde_json::from_str::<EvenV2>(data) {
        return e.into();
    }
    if let Ok(e) = serde_json::from_str::<EventV1>(data) {
        return e.into();
    }
    Err(format!("Failed to parse event: {data}"))
}

Or something similar

cottinisimone commented 1 year ago

Trying to have a macro that auto-versions Events seems like a lot of accidental complexity

I totally agree with this, especially when you say:

how do you handle merge conflicts?

But for the other perspective this means that you need to have all the events version in your codebase. This means that if you have an event (that should be an enum) with 5 variants everytime you change one of the five variants field you need to replicate the entire event enum. I think that this will be a total mess in the long run. On the other hand versioning events with a macro allows you to have a single external file that can grow up in time, can keep the versions for every single variant instead of replicating the entire enum and allows you to have a single enum for the event in your code. Because having different versions of the same event stored in your code will end up anyway in lots of merging problems imho

succoDiPompelmo commented 1 year ago

I give my 2 cents, even though I did not participate in the internal discussion/study group, so do discard my words if I'm missing some context.

if the business logic has actually changed, then you would define a completely new in-memory type that can only be obtained by a proper command -> event flow, and the "old" event is just not compatible with this - e.g. the new email field is mandatory, and the business logics assumes that this data is indeed provided: there just is no way to upcast/transform old events into the new ones (no sensible default exists!), and you require an actual AmendWithEmail command at the domain level to navigate between the two

I agree with Angelo and support his points exception made for the last one. If for example at one point in time business decide the field email become required the business should provide a sensible default even if it not exists. Diving deeper, if we have a website where we registered customer without email but after some time the business decision to require the email we cannot discard the fact that until now all the users have not been able to have an email. We need the business logic to handle this case.

In addition, I vote for Upcasting. I prefer to hide the complexity of managing the versions from the listener, also in the case we have an event that reached the V10 each new listener should implement 10 different versions each time.

angelo-rendina-prima commented 1 year ago

If for example at one point in time business decide the field email become required the business should provide a sensible default even if it not exists. [...] We need the business logic to handle this case.

What kind of default should this be? Some random email? IMHO, if tomorrow Product decides that we also need to collect emails, THEY also have to figure out a way to obtain the missing data (get our agents to telephone the customers and ask for the email directly!) - and this operation is a domain one, not a simple "upcast" that the code has to deal with.