search

spring-projects / spring-modulith

Modular applications with Spring Boot
https://spring.io/projects/spring-modulith
Apache License 2.0
806 stars 138 forks source link

Marking event as completed fails silently #556

Open maciejwalkowiak opened 6 months ago

maciejwalkowiak commented 6 months ago

When event serialization is misconfigured, it may happen that the serialized_event stored in the event_publication table will have slightly different value than the serialized event used in EventPublicationRepository#markCompleted. As a result, even though the event was processed successfully, it is never marked as processed.

This is a corner case, but since I've run into it, it may happen that others run into this issue too.

Here's the repository that reproduces the issue: https://github.com/maciejwalkowiak/modulith-mark-as-processed-issue

There are two classes to look at and I tried to explain what happens with comments:

Once you run the test, you'll see in logs where's the problem.

I believe EventPublicationRepository#markCompleted should throw an exception if no rows have been updated. This would make our serialization mistake obvious much sooner.

odrotbohm commented 6 months ago

Do you think a EmptyResultDataAccessException would be appropriate, or shall we rather create a custom one?

maciejwalkowiak commented 6 months ago

I can't think of any reason why it wouldn't be appropriate.

odrotbohm commented 6 months ago

Testing a fix for this seems more elaborate than originally anticipated. Removing this from the RC1. I think we should be able to ship it in an upcoming bug fix release, though.

maciejwalkowiak commented 4 months ago

Thinking it through again - I think fail fast strategy would work better here. Ideally event should be not processed at all if its it cannot be marked as completed after successful processing - in this case it means that if JSON -> object -> JSON does not produce same JSON as input, the exception should be thrown before processing happens.

Otherwise, an event may be processed multiple times before the serialization issue is addressed by developers.

Other than quite obvious mistake like one provided in example, the difference between two JSON objects can happen if event is not completed, new field gets added to event payload class, new deployment, and then reprocessing is triggered. Perhaps the ObjectMapper provided to JacksonEventSerializer should be configured to skip null fields and empty collections.

cmetzlerhg commented 4 months ago

We ran into the same issue today and it took us quite a while to figure out, what happened.

In our case we could see, that the date formats were different in the serialized event in the database vs. the serialized event in the UPDATE statement.

In the event publication registry we have the following format in our serialized event:

"createdAt" : "2024-06-20T09:20:16.805066546+02:00"

But when the markCompleted method it was serialized as:

"createdAt" : "2024-06-20T07:20:16.805066546Z"

So in that case the update fails, and the event is replayed but never marked as completed. We're now going back to just send IDs in events instead of event carried state transfer. Maybe the resolution strategy to find an event should be improved (or be customizable).

mschaidinger commented 3 months ago

We ran into the same issue today and it took us quite a while to figure out, what happened.

In our case we could see, that the date formats were different in the serialized event in the database vs. the serialized event in the UPDATE statement.

In the event publication registry we have the following format in our serialized event:

"createdAt" : "2024-06-20T09:20:16.805066546+02:00"

But when the markCompleted method it was serialized as:

"createdAt" : "2024-06-20T07:20:16.805066546Z"

So in that case the update fails, and the event is replayed but never marked as completed. We're now going back to just send IDs in events instead of event carried state transfer. Maybe the resolution strategy to find an event should be improved (or be customizable).

Thank you so much for pointing me in that direction. It would have taken me ages to locate the problem. I fixed it with Jackson configuration for the time being:

spring.jackson.deserialization.adjust-dates-to-context-time-zone=false