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

Typed persistence: register multiple onRecoveryCompleted callbacks #25428

Closed
svroonland opened this Issue Aug 2, 2018 · 4 comments

Comments

@svroonland
Copy link

commented Aug 2, 2018

This is a request for the typed persistence API to make the PersistentBehavior a bit more composable.

Currently, calling onRecoveryCompleted replaces any existing callback, so something like the following is not possible:

val behavior = PersistentBehaviors.receive { ... }
  .onRecoveryCompleted(...)

// Somewhere else, decorate the behavior with another recovery callback
behavior.onRecoveryCompleted(...)

Having onRecoveryCompleted chain existing callbacks would be really nice to support this. I suppose the same applies to onSnapshot.

@patriknw patriknw added this to Backlog in Akka Typed Aug 16, 2018

@patriknw patriknw moved this from Backlog to Ready for production backlog in Akka Typed Dec 11, 2018

@johanandren

This comment has been minimized.

Copy link
Member

commented Feb 1, 2019

After pondering this a bit I think that it would be bad to chain in the on methods because there are no obvious semantics around which callback runs first, what happens if one callback fails etc.

Therefore I'm leaning more towards being able to access the previous defined callback if there is one and compose any way you want with an additional function. Something like:

val previous: Option[State => Unit] = persistentBehavior.onRecoveryCompletedCallback
val withComposedCallback = persistentBehavior.onRecoveryCompleted { state => 
  // here we can do things in any order we want, catch exceptions etc.
  otherCallback(state)
  previous.foreach(cb => cb(state))
}

Wdyt about this? Ping @patriknw

@johanandren johanandren self-assigned this Feb 1, 2019

@johanandren johanandren moved this from Ready for production backlog to In Progress in Akka Typed Feb 1, 2019

@patriknw

This comment has been minimized.

Copy link
Member

commented Feb 22, 2019

Those callbacks have been nagging me for another reason. They are a different kind of API than we have for ordinary actors (we don't use much callbacks). I wonder if we should make them look more like signals, i.e. a signal message you receive.

      EventSourcedBehavior[Command, Event, State](
        persistenceId = PersistenceId(id),
        emptyState = State(Nil),
        commandHandler = commandHandler,
        eventHandler = eventHandler)
        .receiveSignal {
          case RecoveryCompleted(state) => ...
          case RecoverFailed(ex) =>
          case SnapshotCompleted(meta, result) =>
        }

Composition would be possible in similar way as Johan suggests, i.e. that the registered signal handler is accessible and can be delegated to with for example .orElse.

That would also make it easier to add more such signals in the future in the javadsl, which is an abstract class where we can't really add more methods.

@johanandren

This comment has been minimized.

Copy link
Member

commented Feb 22, 2019

I think that's a pretty neat idea

@patriknw

This comment has been minimized.

Copy link
Member

commented Feb 22, 2019

Then it could also support ordinary Terminated message this way.
BTW I recently added preRestart and postPost hooks and those should ofc also be like this.

johanandren added a commit to johanandren/akka that referenced this issue Mar 12, 2019

Akka Typed automation moved this from In Progress to Done Mar 12, 2019

johanandren added a commit that referenced this issue Mar 12, 2019

@johanandren johanandren added this to the 2.5.22 milestone Mar 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.