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

ReadModelPopulator is incompatible with IReadModelContext.MarkForDeletion() #845

Closed
alasdair-stark-screenmedia opened this issue May 26, 2021 · 9 comments
Labels

Comments

@alasdair-stark-screenmedia

I have a read model which calls MarkForDeletion() when it gets into a state where it does not contain any useful information. The idea is to keep the read model table tidy and minimise storage requirements. Another benefit is that we can make certain inferences if a record is not present in the read model table.

I am attempting to use the read model populator to rebuild a read model. I have noticed that the batching functionality appears to build the read model record from all applicable events in the batch before writing the record to the database.

The problem is that after MarkForDeletion is called, another event in the batch could update the read model with meaningful information. In normal operation, the readmodel would be inserted again. However, when using the readmodelpopulator, the readmodel will simply be deleted because there is no way to 'undo' MarkForDeletion.

The only solution seems to be setting PopulateReadModelEventPageSize to 1 but this obviously degrade the performance.

@WardenUnleashed
Copy link
Contributor

This issue is related to an issue I posted around 8 months ago and has an impact on Read Models even when you aren't repopulating them with the ReadModelPopulator. #792.

@rasmus I think adding this capability to 1.0 or a future release would be another good wishlist item.

@CentinelaBot
Copy link

Hello,

I am not sure if I understand the problem, from what I have read you are using MarkForDeletion in your reading model.
The behavior you should take would be to remove the record completely from the read model.

I understand that if you then want to repopulate the read model from the events, the behavior would be that when it encounters the event that triggers the MarkForDeletetion it deletes the record again.

I am not sure if this is what you think or it is another problem?

@alasdair-stark-screenmedia
Copy link
Author

@WardenUnleashed is correct, it's the same issue.

@WardenUnleashed
Copy link
Contributor

I am not sure if I understand the problem, from what I have read you are using MarkForDeletion in your reading model.
The behavior you should take would be to remove the record completely from the read model.
Right, the issue is when you call MarkForDeletion , it keeps the read model permanently deleted despite events after the event being applied. This occurs because there is no UnmarkForDeletion equivalent that will allow for the read model to be re-created.

A pretty common scenario I have encountered has been having an event that will "delete" and another one to "restore" that comes after the delete event as a mechanism to undo that. Since there is no way to unmark the read model for deletion once MarkForDeletion has been called, this has forced us to add a boolean to most of our read models that represents whether the read model is deleted or not. Not an ideal solution for sure.

@CentinelaBot
Copy link

I am not sure if I understand the problem, from what I have read you are using MarkForDeletion in your reading model.
The behavior you should take would be to remove the record completely from the read model.
Right, the issue is when you call MarkForDeletion , it keeps the read model permanently deleted despite events after the event being applied. This occurs because there is no UnmarkForDeletion equivalent that will allow for the read model to be re-created.

A pretty common scenario I have encountered has been having an event that will "delete" and another one to "restore" that comes after the delete event as a mechanism to undo that. Since there is no way to unmark the read model for deletion once MarkForDeletion has been called, this has forced us to add a boolean to most of our read models that represents whether the read model is deleted or not. Not an ideal solution for sure.

I can only think of the logical elimination. If you wanted to do what you intend to do it would be against the way one would expect after marking a MarkOfDeletion I can't think of unmarking the record to restore it after having marked it for physical deletion. Wouldn't it be confusing?

@WardenUnleashed
Copy link
Contributor

WardenUnleashed commented Jun 9, 2021

I can only think of the logical elimination. If you wanted to do what you intend to do it would be against the way one would expect after marking a MarkOfDeletion I can't think of unmarking the record to restore it after having marked it for physical deletion. Wouldn't it be confusing?

I don't think so. IsMarkedForDeletion has a property called IsMarkedForDeletion, which MarkForDeletion sets to true.

The behavior I expect would be that if IsMarkedForDeletion is true after applying all events relevant to the read model, then the read model will be deleted. Conversely, if IsMarkedForDeletion is false after applying all events, I would expect for the read model to be created or updated.

Whether IsMarkedForDeletion is true or false during the applying of events(and what should happen as a result of that) can and should be handled through the read model's event handlers.

It doesn't seem to me like marking it for deletion needs to be a one way action and adding the capability for a read model to no longer be deleted would be super helpful.

@AnthonyCarl
Copy link
Contributor

AnthonyCarl commented Jul 8, 2021

@Teides I don't think there should be an UnmarkForDeletion either. The problem is that the behavior is different in the "rebuild" code path, which applies events in batches, vs the "normal" read model update which applies event one by one (which is why @alasdair-stark-screenmedia solution works). In the one by one case, the model would have actually been deleted in the read store. In the batch case, the model stays in memory during processing and never gets "deleted" mid batch.

The way I see it, if the context is ever marked for deletion, the populator should "delete" it in memory and start over if further events would manipulate it (including a context that is not marked for deletion as the model would in reality be "new" at that point since it was recreated).

Here is the code that will delete the row in SQL even if the the row would be recreated later in the "normal" one by one flow. You can see that if the model was ever marked for deletion, it gets deleted. This is regardless if later events would recreate the row. To put it another way, the core batch application assumes you would never recreate the model/row after you said you wanted it deleted. Effectively, the only reliable delete in a batch operation is a delete caused by the final event in the batch. Otherwise, your read model state will most likely be incorrect.

In the ReadModelDomainEventApplier, the same IReadModelContext is used for all operations against the batch of events. Therefore, once MarkForDeletion() is called, the record will be deleted regardless if subsequent events are applicable.

It would seem to me that the read model should be recreated "new" in memory and the read model context "reset" when an event is encountered that would recreate the record after deletion. Perhaps this is functionality the ReadModelEnvelope<T> could handle. Or maybe there needs to be some other object created for this purpose to properly track the current in memory sate of the read model until it is persisted. This object should handle the case the read model is deleted and recreated in a batch operation. The final disposition should also be correctly calculated, whether it be deleted, modified, or created new in the data store.

I have dug into the problem off and on and the work around of soft deleting as mentioned by @WardenUnleashed is generally how we work around this issue since fixing this issue would require a change to the core EventFlow library.

@github-actions
Copy link

github-actions bot commented Apr 8, 2023

Hello there!

We hope you are doing well. We noticed that this issue has not seen any activity in the past 90 days.
We consider this issue to be stale and will be closing it within the next seven days.

If you still require assistance with this issue, please feel free to reopen it or create a new issue.

Thank you for your understanding and cooperation.

Best regards,
EventFlow

@github-actions github-actions bot added the stale label Apr 8, 2023
@github-actions
Copy link

Hello there!

This issue has been closed due to inactivity for seven days. If you believe this issue still
needs attention, please feel free to open a new issue or comment on this one to request its
reopening.

Thank you for your contribution to this repository.

Best regards,
EventFlow

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants