riker-rs / riker

Easily build efficient, highly concurrent and resilient applications. An Actor Framework for Rust.
https://riker.rs
MIT License
1.02k stars 69 forks source link

Is there a way to send a message back to sender after persisting? #43

Closed jpopesculian closed 5 years ago

jpopesculian commented 5 years ago

I understand how I would return messages to a non actor with the ask pattern, but is it possible to do that after persisting?

  fn receive(&mut self,
                ctx: &Context<Self::Msg>,
                msg: Self::Msg,
                sender: Option<ActorRef<Self::Msg>>) {
        ctx.persist_event(msg);
    }

    fn apply_event(&mut self, _ctx: &Context<Self::Msg>, evt: Self::Msg) {
       // how could I access `sender` here?
    }
jpopesculian commented 5 years ago

44

Added a pull request for reference of maybe what that would like. Unfortunately a breaking change

leenozara commented 5 years ago

Hi @jpopesculian thanks for your contribution. I know that others are including sender in the event and then not serializing the actor ref, leaving it as None when it is replayed. That isn't quite right and I like your approach to this. I wonder about naming here. Is sender accurate, or something else make sense?

@kitsuneninetails I remember you were using a flow similar to @jpopesculian and had some input around this.

jpopesculian commented 5 years ago

@leenozara yeah I just super quickly threw that together, but I agree the sender name is a little confusing, especially since its being passed around alot. just wanted to get your input on it, and I'd be happy to change it as necessary if its something you see could be useful!

kitsuneninetails commented 5 years ago

It looks like the idea of the PR is to enhance persist_event to take a single sender. I'm guessing this will work, since the actor ref is passed along with the event. Using a single Option object, etc. in the struct doesn't work, because another event can be persisted, which would cause any sender ref option in the main struct to be over-written.

Another way to accomplish this (if you aren't trying to reply to asker actors) is through actor Channels and broadcasting notice messages in a channel. Your target actors can be listening and accept this broadcast. This won't work if you are trying to reply to an ask actor, however.

In my case, I didn't want to make a breaking change, and I did indeed need to reply to the ask actor, so I added a Hash Map of actor ref IDs to actor refs on the actor struct itself. I add the ref of the actor I would like to send a message to in this map (keyed on ref ID) during the main message handling (before I call persist_event, and fetch it during apply_event. It works without a breaking change, but I'm thinking if a change were to be introduced, this PR seems like a good way to do this.

As for the naming, as Lee pointed out, this could be ANY actor ref passed in at will from the actor; and in fact, should it be limited to only one? But it indeed seems to be mostly useful in exactly the situation I had, where I was trying to hold off replying to the ask actor (and sending an HTTP response back via my HTTP server) until after the event was persisted. In that use case, sender does seem appropriate IMHO.

jpopesculian commented 5 years ago

I still think there's power in allowing the user to use the sender after persistance... but I actually just implementing a kind of meta object on all protocol messages that keeps track of a stack of ActorRef's so that I can return up the stack when necessary. I'm honestly new to Actors and things, so I don't know if that's the best pattern for all of this but it made sense to me. As the sender list isn't used in replay events, it just deserializes to a blank list....

leenozara commented 5 years ago

I like @jpopesculian 's separate function parameter for passing in an ActorRef. If we wanted to expand this to pass a 'stack' or 'context' that can contain multiple ActorRef what would that look like? If that makes sense, then where else other than apply_event would that also apply?

danmerino commented 5 years ago

This makes a lot of sense to me. I might be naive but I think the ActorRef is all that is needed @leenozara

This PR might collide on a line a bit with https://github.com/riker-rs/riker/pull/40 which is a one line fix... but that PR is critical in my opinion. If anything, it simplifies this PR and makes replay not have side effects, which it has right now.

jpopesculian commented 5 years ago

@leenozara sorry that I'm taking a look at this so late. I mean I came up upon kind of a conceptual difference between someone waiting on a reply and the sender. For example:

REST Endpoint
     |
     v
   Actor [ sender: Endpoint, waiting: Endpoint ]
     |
     V
   Child [ sender: Actor, waiting: Actor ]
     |
     V
   Child [ sender: Child, waiting: Actor ]
     |
     V
   Actor [ sender: Child, waiting: Endpoint ]
     |
     V
REST Endpoint

By the way, I think #40 is much more critical and ready to be merged...

jpopesculian commented 5 years ago

Again, I'm new to event sourcing, I don't know if this is the right way to be looking at this...

leenozara commented 5 years ago

Hi @jpopesculian thank you for being so patient and my apologies for letting this slide for so long. I'm going to get this merged in this week. I've been reluctant to do so up until now because there's some fairly significant changes on a separate branch that impact how sender ActorRef is used in general. Work on that branch is taking much longer than I expected and it's already been too long to get your contribution merged - so I'll get this done this week - thanks!

jpopesculian commented 5 years ago

Hey no worries @leenozara ! Let me know if you need help with anything, I like the project! Again I solved this problem personally, so I don't find the change as critical but #47 should be fixed as soon as possible. Thanks for all the good work!