Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Command handling and version conflicts #110

Open
alexeyzimarev opened this issue Jun 24, 2022 · 2 comments
Open

Command handling and version conflicts #110

alexeyzimarev opened this issue Jun 24, 2022 · 2 comments
Labels
enhancement New feature or request

Comments

@alexeyzimarev
Copy link
Contributor

The current command handling flow looks like this:

  • Load the aggregate
  • Call the model
  • Persist new events

When the last step fails due to an optimistic concurrency exception, an external policy can retry the operation because the domain models to retry blindly. However, the stream is loaded again, because the retry happens on the outside. Also, we need to read the stream to get new events that caused the conflict.

However, it's possible to react to the optimistic concurrency exception by reading the stream tail from the last known event version. For shorter streams, the difference would be marginal, but for streams longer than a hundred events it would presumably be faster as we'd only need the stream tail, which most probably only has one event that caused the concurrency exception.

A prerequisite for this would be that the aggregate object contains the list of original events in addition to the list of new events as it does today. Opening up the list of original events for access has other benefits that I would be describing here.

I guess that the optimistic concurrency retry needs to be optional as it's not always the desired default behaviour of the model. The question would be if it should be the default or not...

@alexeyzimarev alexeyzimarev added the enhancement New feature or request label Jun 24, 2022
@bartelink
Copy link
Contributor

A prerequisite for this would be that the aggregate object contains the list of original events in addition to the list of new events as it does today. Opening up the list of original events for access has other benefits that I would be describing here.

If you can stash the state and the version, then you don't need the events (and don't need to deserialize or fold). That also works better with snapshots etc. (Of course that does assume that you can clone the state and/or that the state is a persistent data structure)

The question would be if it should be the default or not...

You also want a max retry count - perhaps an optional arg defaulting to 1?

@alexeyzimarev alexeyzimarev moved this to Todo in Eventuous Nov 4, 2022
@alexeyzimarev alexeyzimarev moved this from Todo to Triage in Eventuous Nov 4, 2022
@alexeyzimarev
Copy link
Contributor Author

Basically, the list of original events is already there, added for a different purpose. So, I have more information than I even need.

Avoiding deserialization only helps in scenarios when the domain logic only uses the state for decision-making, and today it's possible to use original events for that purpose without folding events. It, of course, makes using things like Spyglass hard, as the state isn't there... I guess it's up to the developer to decide which way to go.

Btw, I just realised that #157 might be useful for this issue. But, as I plan to make caching optional, there must be something in place to work without caching too. I guess the code would be similar - there's an instance, but it's obsolete, and need new events to get up-to-date.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Triage
Development

No branches or pull requests

2 participants