He-Pin commented on code in PR #3655: URL: https://github.com/apache/pekko/pull/3655#discussion_r4357192010
########## docs/src/main/paradox/typed/durable-state/persistence.md: ########## @@ -96,6 +96,12 @@ The two most commonly used effects are: * `persist` will persist the latest value of the state. No history of state changes will be stored * `none` no state to be persisted, for example a read-only command +The state is typically an immutable class, with a new instance passed to the `persist` effect. +When using mutable state, the command handler may update the existing state instance, but it must still Review Comment: [P2] This in-place mutation advice is unsafe with the current durable-state reset behavior. Recovery without a stored value uses the same instance retained as `setup.emptyState` (`state/internal/Recovering.scala:163-165`), and delete resets to that instance (`DurableStateStoreInteractions.scala:74`). Mutating the first live state therefore also changes the value that delete restores. I reproduced `initial=0 -> mutate and persist 1 -> successful delete`: the state after deletion was still 1. With the same actor/store/probe and a fresh instance passed to persist, deletion correctly restored 0. Please keep fresh/immutable-state guidance here, or first fix the reset semantics and cover deletion and failed-first-write recovery before recommending general in-place mutation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
