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
Reset effect in durable state. #30446 #31071
Reset effect in durable state. #30446 #31071
Conversation
* Add reset effect to the state dsl. * The effect calls deleteObject in the durable state store. * The effect updates state to the empty state.
Hi @jausmann-wc, Thank you for your contribution! We really value the time you've taken to put this together. Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for bringing this missing feature forward
case Failure(cause) => InternalProtocol.DeleteFailure(cause) | ||
} | ||
|
||
Running.RunningState(revision = 0, state = setup.emptyState, receivedPoisonPill = false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We must also think about the query side. A new DurableStateChange
type must be added as was prepared for in https://github.com/akka/akka/pull/30549/files#diff-43c5d2b2092ca2bc90e24d02701f2494fa9dbb951f396f94cc44738395a8efa1R16
I'm not sure we should reset the revision to 0. Then the next state will be stored with revision 1 and that might be treated as a duplicate, at least that would be problematic in https://github.com/akka/akka-persistence-r2dbc
Maybe we should just continue the the revision numbers even after reset? It would be up to the plugin implementation to keep track of latest used revision number. We already have that requirement for sequence numbers for event sourced.
The way I would implement the deletes in the r2dbc plugin is to not actually delete the row but just clear the payload column. That is similar to how it's done for events. In that way it's easy to continue the sequence numbers even though all events have been "deleted".
/** | ||
* Reset to the initial state and delete the persisted state. | ||
* | ||
* Side effects can be chained with `thenRun` | ||
*/ | ||
def reset[State](): EffectBuilder[State] = Reset() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if it's not better to call it delete
instead. The fact that a reset happens is just a consequence of the deletion and so don't need to be explicit in the API.
What I mean is that if we delete a row, we can always add it again assuming the primary key is not an auto-increment one. If we send a new create command for that same entity id, we will by design get the initial state and it will behave as if it was the first time that we call it.
If I don't want a deletion, but real reset. Then I can simply have a command that ignores the existing state and pass the initial state to persist
. So, reset
is easily implementable by the users.
Also, an explicit deletion will fit better with the query issue raised by Patrik in his comment https://github.com/akka/akka/pull/31071/files#r788886575
* Do not increment the revision when deleting. * Implement DeletedDurableState for persistence query. * Update PersistenceTestKitDurableStateStore so that deleteObject sets the Record payload to None, ie delete the payload.
Hi @jausmann-wc, Thank you for your contribution! We really value the time you've taken to put this together. Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement: |
Continued in #31099 |
#30446
References #30446