jodal / pykka

🌀 Pykka makes it easier to build concurrent Python applications.
https://pykka.readthedocs.io
Apache License 2.0
1.22k stars 107 forks source link

Reverse actor stopping order (was: Check that actor is still runable after message has been recieved.) #8

Closed adamcik closed 13 years ago

adamcik commented 13 years ago

On line https://github.com/jodal/pykka/blob/master/pykka/actor.py#L164 the check for the run-able state is done before inbox.get() which can block for any amount of time, during this waiting run-able can very well have changed.

Symptom of this problem is not finding the refs that you expect in the actor registry. This happens easily when stress-testing with a lot of clients in my current mopidy branch. For each new connection a ThreadingActor is started and as socket data is received it is delivered as a message. Then I CTRL^C mopidy and it stops all the actors, however, since some of my actors have messages in their inbox still they will go on to handle them despite being stopped. Part of this handling includes looking for other non-running actors which of course fails.

adamcik commented 13 years ago

Hmm, problem could also be due to simple thread switching, as I'm testing with while sleep 0.1; do mpc status &; done it is very much possible that a thread that has started running loses its established actor proxy midway. Not sure if this really can be solved within pykka.

adamcik commented 13 years ago

«The actor will finish processing any messages already in its queue before stopping. It may not be restarted.» - so I guess this is intended behavior. So, I guess my new question is, how def _run(self): manages to keep this promise. Based on my understanding of that pykka code, it would only handle the last message received before being set to not run-able.

jodal commented 13 years ago

When using ThreadingActor, the actor inbox is a Queue.Queue, which is a FIFO queue. Thus self.actor_inbox.get() will first return all messages queued before the stop message, then the stop message.

By using Queue.PriorityQueue, we could change the implementation to give stop messages higher priority than regular messages, and thus stop the actor faster, with probably less probability of the actor processing a message which requires interaction with other already stopped actors, causing an error before the stop message is processed.

To shut down actors in a sequence that takes dependencies into consideration, avoiding the case where living-soon-to-stop actors depends on stopped actors, I think we would need some kind of supervision hierarchy. If we're to go that way, we should probably have a second look at Erlang and/or Akka to see how this can be solved once and for all.

adamcik commented 13 years ago

I was thinking about rewriting mopidy's stop_all code to do frontends/lsiteners before backends, would probably be a fairly simple way of starting to improve/avoid this for mopidy's case.

jodal commented 13 years ago

For the records: after some discussion on IRC, we've landed at a simple solution that will make ActorRegistry.stop_all() work in simple cases:

In non-simple cases, the application developer will have to shut down actors in the correct order himself. This should be documented.