-
Notifications
You must be signed in to change notification settings - Fork 23
Akka Typed: API discussion #46
Comments
We also noted a few other things (please fill in with your notes).
|
Some of my comments and notes:
+100, it really feels like a Props - stuff that accompanies "the actor". We also contemplated putting restart things into the Props, so yes it would be synchronous but in failure restarts would be in control of the one that starts the Actors. This is specifically very nice for things that start Actors for users like cluster stuffs and persistence where we (or just users) may want to add exponential backoff supervision (I think that's one of the more important changes we can do).
Slightly in favour of this. Since it's not as easy to get things wrong (closing over), and people use ask anyway this can make ask simpler to people. I also think
Other questions:
Will comment some more pretty soon, heading out for now. Very exciting to get hands and personal on with this finally! |
A few thoughts (from the viewpoint of not having actually had a chance to play with it, so take or leave these as seems appropriate):
Is there a better opportunity to say "do this before I accept messages" in the new world? That's the real need, as I see it.
I'll just note that having it separate is precisely why Requester works as well as it does, as a nearly drop-in replacement for (Actually, in my perfect world, Requester would be the standard behavior -- the Actor would provide a Requester-style ExecutionContext, and that's how Futures would work by default. But I'll admit that I haven't looked hard enough at the pros and cons of that.)
Is it that much less common to close over mutable state in the new architecture? I'm willing to believe it, and losing Actually, here's a question that occurs to me: are there any examples of the interaction between In general, though, this is looking really exciting. Going to take a while to get the new style into my head, but I love the example code, and am already chewing over the problems in my existing system that would be solved by the new model. (A nice example from my own code, in case anybody's tracking use cases: while the ideal is Run-Anywhere, in practice that's not always practical. Querki is intentionally built so that the local troupe of Actors around a given Space can share the SpaceState -- a huge immutable structure that I do not ever want to serialize over the wire. One problem I've had in practice is distinguishing whether a command is coming from that local troupe (and thus can receive the updated state in the response) or from outside (so it can't). That's currently painfully ad-hoc, but in the new world I should be able to distinguish "local" and "potentially remote" channels in a lovely and principled way.) |
I would have loved to join you in Vilnius! But I guess one cannot always win … First the good ones:
withContextI sympathise with the concept of unifying context extraction with Deferred, but I disagree with the premise. The current DSL is designed to minimize coupling between Behavior and ActorContext, to factor these concerns of description and execution cleanly into different parts. Syntax wise adding ask methods to ActorRefI’d much rather remove ask completely, at least the Future-based version. Within an actor I don’t see a good reason to use ask, ever:
bubbling up failuresUnhandled child termination will bubble up if watched—if not watched then obviously the parent does not care. This simplifies things to a single concept that also makes sense semantically. taking PartialFunction argumentsIn general I dislike requiring PartialFunction in signatures because it means that client code is forced to use case statement syntax, which is not otherwise a given in all cases. The argument of extensibility is a good one, though: the Signal trait should most definitely not be sealed! Opaque BehaviorThe beauty of the current model is that it expresses exactly the nature of an Actor: a behavior is a function from input to new behavior. Having two methods is owed to the lack of union types in Scala 2. If Behavior became opaque, it would be impossible to write new ones without the overhead imposed by the current ones (i.e. requiring extra closures to be allocated and executed). One example is the process DSL. It is not clear to me what the advantage would be of switching from a function to something that needs an interpreter. Being able to call DeploymentConfig vs. PropsThe reason why I chose that name is that Props have a different connotation, they used to wrap a behavior (coming from untyped). Would it not be confusing to users? If my concern here is unwarranted, then of course the rename is good, I’m always for shorter names. RestarterThis part is not clear to me: the API already is in the DSL packages, as is appropriate, and since both languages share a common implementation that is put elsewhere. The implementation could move to the But I don’t understand the comment of wrapping at the wrong level: as far as I can see Restarter is a decorator that can be added many times, at all levels. What is wrong with that? In particular, every instance of it catches a specific type of exception, so some layering is needed. And with the current scheme an actor that accepts foreign Behaviors can easily wrap them in a Restarter, no need to make that part of the Props. One thought here is that it might make sense to have a non-sticky variant of Restarter, so that the actor itself could declare part of its operations as restartable. recursivelyIt took me a few minutes to figure out what the signature means, which I think is a rather bad sign in terms of usability for newbies. The benefit would obviously be that the state is reified instead of implicit, allowing better debug logging in tests. A signature that would be simpler to understand is def stateful[T, U](initial: U)(step: (U => Behavior[T], U, T) => Behavior[T]): Behavior[T]
// or even
def stateful[T, U](initial: U)(step: (U, T) => Either[U, Behavior[T]]): Behavior[T] Still, I’m on the fence whether this should be in the standard package. People can easily write this themselves (with extensible Behavior!) or copy from some gist. GeneralI would have loved to hear why |
(More from the peanut gallery. Y'all feel free to tell me to quiet down if it's too much.)
... fascinating. I'd missed your post on the process DSL (I just looked it up), but if I'm understanding it correctly, I agree with your point. Futures inside of Actors have always been, at best, a barely-necessary evil, and eliminating (or taming) them inside a better abstraction would be a huge win. Am I correct that, in the process DSL, the combination of IMO the DSL's syntax is a tad clunky for everyday use, and I'd like to better understand how it would work in the context of an ongoing stateful Actor. In particular, the model I want all the time is:
If that's achievable, I suspect I would be happy to see Very neat stuff, and I love the composability... |
Thanks a ton for the elaborate replies, that's the kind of discussions I love :-)
I think the worry about confusion is overprotective to users. I think that users mostly think of Props as "the additional actor config stuff". I have not met a person outside the team that would notice that the primary goal would be the factory trick - since we've hidden it in the I think the rename is safe and useful enough to warrant doing it :-)
IMHO the I was also thinking about calling anything that gives space to "keep a mutable var" simply "Mutable { var x = ???; { case ... => ... } }` if you know what I mean? I really would like to avoid the My personal "dream" API is this by the way (core of it at least, which basically combines the Stateful and Deferred, and would add some overloads or conversions to allow "if I want context I just add This is the draft I had, for inspiration (i had it working, but impl was very very sub optimal, since it would allocate function for every message receive): // ---- FOR INSPIRATION ----
// while I love the "Actor" name here, perhaps NOT what we want as it's confusing?
final case class Actor[T](receive: ActorContext[T] ⇒ T ⇒ Behavior[T])
extends Behavior[T] {
override def management(ctx: ActorContext[T], msg: Signal): Behavior[T] =
Same
override def message(ctx: ActorContext[T], msg: T): Behavior[T] =
receive(ctx)(msg)
}
// implicit def hakk[T](f: T ⇒ Behavior[T]): ActorContext[T] ⇒ T ⇒ Behavior[T] =
// ctx ⇒ t ⇒ f(t)
implicit def hakk[T](f: PartialFunction[T, Behavior[T]]): ActorContext[T] ⇒ T ⇒ Behavior[T] =
ctx ⇒ t ⇒ {
if (f.isDefinedAt(t)) f(t)
else Unhandled
}
sealed trait Model
case class Greet(who: ActorRef[Greeted]) extends Model
case class Greeted() extends Model
def lol(take: String) = Actor[Model] { ctx ⇒
{
case newName: Greet ⇒
lol(take) // TODO WHAT DOES THIS MEAN
case changeName: Greeted ⇒
Same
}
}
Actor[Model] {
case newName: Greet ⇒
Same
case changeName: Greeted ⇒
Same
}
Actor[Model] {
model ⇒ Same
}
// the above works because in Behavior object we could:
// implicit def same_2_same[T](b: Behavior[Nothing]): T ⇒ Behavior[T] =
// null // FIXME, can we figure it out? |
@ktoso We are talking about a handful of characters of boilerplate code. For purpose of ignoring naming for now, assume a factory method makeActor[Command] {
case (ctx, Ping) => // send pong
}
// vs
makeActor[Command] { ctx =>
{
case Ping => // send Pong
}
} To me the cost of making this magic work consistently is just too high (not to mention the reason for that funny empty line and all the allocations; with the current DSL users can decide to spend a few extra characters on a Again, we can experiment with alternative ways of doing things, there can easily be a plurality of solutions in this space. My preference is to keep the first solution that we offer clean, minimal, and dead obvious. And of course, Behavior must remain extensible. |
Our idea of how to implement was to let the machinery detect the returned RK: Yes, I got that part, but the syntax would invite mistakes like the following case Ping =>
withContext { ctx =>
// do stuff, create actors, whatever
}
Same It is enough to have |
suffers from tuple allocation without most users knowing (but I'm not sure it's a big deal) RK: yes, as I said, the user can choose aesthetics over performance, and we know from Free monad usage that many people just don’t care about execution characteristics |
That would require usage of RK: Yes, true, thanks for making this explicit. |
scala> object A { def f(x: Int => Int)=42; def f(x: (Int, Int) => Int)=43 }
defined object A
scala> A.f(x => x)
res3: Int = 42
scala> A.f((x, y) => x)
res4: Int = 43 So we can actually overload EDIT: well, that does not work with partial function literals, so separate names would be needed to support those |
Yes I think this is indeed the direction of what we're (I'm) after.
To clarify: The funny empty line requirement does not seem to exist AFAICS? Just checked and it compiles on 2.12.
Though one could also look at it that way that most of the time context will likely not be used yet we keep matching on it. Though the Perhaps it is fine though... You may be right that the
Absolutely! Which is why I'd want to bounce some more idea before settling on the API - even if you have bounced it back and forth some time, we have not had much time hands on with it yet and may come with new observations/options. So the => => I think we drop, but back to the context hm |
Just my uneducated 2cs: the new behavior system allows for various composition patterns, hence I think the goal should be to have the most flexible, and still reasonable base-line implementation, then one can add different styles of APIs on top. I would personally prefer if one can do things allocation free if necessary. |
That's a good argument. I have been on the fence but this makes me tip over to that we should not provide |
Conclusions:
If you agree I will create separate tickets for the tasks. |
Sounds very good, with one exception: I think PreStart can and should be dropped because using Deferred is the right thing to do. When creating actors during construction, the resulting behavior will need to differ from the initial one in that it now has references to those actors; this makes PreStart a signal that is handled exclusively to then create the real behavior—Deferred currently is a shorthand for this. The issue I have (which is still reflected in some TODOs) is that it is easy to create something that is not composable with this approach because it forgets to inject a PreStart as the first message. Using Deferred for all these purposes would solve this elegantly—I don’t know why I didn’t see that previously ;-)
In that light, Deferred should not be renamed to Mutable because it will often be used in a completely immutable fashion, just deferring things until instantiated. If you find a better term that conveys this meaning, then I’m all ears, but so far Deferred is the one that best matches the semantics.
|
Hmm, I'm not yet sure. I didn't really explained the rationale behind the combinators as shown above and just dumped them there (sorry for that). Not all of them are necessary (like Here's basically the rationale for what we did: We can approach the space either from the functional or from the side-effecting direction. The pure, functional way would model all effects explicitly (including those provided by the context and message sending) as the return type of the handler. Effects could be composed to a composite return value. Whatever runs the handler will "interpret" the effects and execute them. The side-effecting is very much the old API where all side-effects are achieved directly by mutating internal directly or indirectly. I think we all agree that the purely functional way of doing things would be slow because too many wrapper objects are being created. I think we also agree that we should provide a user API both for the functional and the mutable way of specifying actors. The current approach of akka-typed in master is already a mix of both:
The functional part (= changing the state) is already now achieved by interpreting the result of the handler. The result can either be
The summary of that is that we already now have an interpreter in place that is spread over multiple places. Regarding the "combinators" to provide, the problem I see with When we tried to achieve that we ended up with only the two main building blocks shown above, E.g. if you want to build a behavior, i.e. one state that is part of a larger actor state machine, that spawns a child actor and then waits for three messages and sends them to the child in a batch: // new style
def getThreeAndSendToChild: Behavior[String] =
withContext { ctx =>
val child = ctx.actorOf(...)
var batch: List[String] = Nil
handleMessages {
case msg =>
batch ::= msg
if (batch.size == 3) {
child ! batch
batch = Nil
}
Same
}
}
// example usage:
def fooActor: Behavior[String] =
handleMessages {
case "init" => getThreeAndSendToChild
case _ => Unhandled
} How would you build that behavior with the current API? Maybe like the following. We cannot use // current style
def getThreeAndSendToChild(ctx: ActorContext[String]): Behavior[String] = {
val child = ctx.actorOf(...)
var batch: List[String] = Nil
Stateful { (ctx, msg) =>
batch ::= msg
if (batch.size == 3) {
child ! batch
batch = Nil
}
Same
}
}
// example usage:
def fooActor: Behavior[String] =
Stateful {
case "init" =>
getThreeAndSendToChild(ctx)
Same
case _ => Unhandled
}} but whoops, now we have the same problem even with the existing API, that was mentioned before:
So, I see the danger, though I'm not sure if that's really a practical problem. In a real example you would have to return a behavior inside the block as well. Wouldn't people notice that they return behaviors at two different places? Could the name be improved to prevent that or could we add runtime checks that prevent such a usage? |
I don't understand that completely. How would you propose that the |
Johannes, I am not sure how, but we are definitely talking past each other. Deferred and withContext have exactly the same signature and semantics, do they not? Where is the dichotomy?
|
The current |
It would be interpreted in a special way same as |
Ah, I see, we are splitting hairs ;-) Because
So, if these are unconvincing to you, then I’d also be fine with the current withContext proposal if the factory is renamed to Deferred :-) On the interpreter, @jrudolph has a good point in that it is currently easy to mistreat a Behavior. Perhaps we can have the best of both worlds using a |
@rkuhn I only saw your comment regarding Regarding the |
Excellent! |
Does that mean that we remove the ctx parameter from the Stateful, or do you still want to keep that? |
Not offering the binary signature means that every access to the ActorContext necessarily involves an extra closure somewhere. The most composable, encapsulated way would then need to allocate a new closure for every message(*), since the We should try whether both—unary and binary—can be offered. (*) Actually, I think it can be avoided, but the code will not look as nice as it otherwise could. |
I have created tickets |
Today we reviewed the current typed Actor API for Scala. We tried to improve a few things:
msg match
everywhere (which is needed because the callbacks are defined with 2 parameters, context and message)Deferred
(which would be mostly used for behavior definitions relying on mutable internal state) with the more functional API (likeStateful
)We attempted a prototype at how it could look like. The basic ideas are those:
withContext
as the single way to access the contextBehavior
is now opaque to the user, internally different subclasses are supposed to be used, whatever runs the behavior will have to distinguish these subclasses (like it already does now forSame
andUnhandled
)preStart
signalWe reimplemented the chatroom example using different kind of styles using the new API. We currently favor the
chatRoomMutable
andchatRoomFunctional
kind of doing things. (We understand that thechatRoomFunctional
example will use one extra lambda allocation to supportwithContext
which might be fine.chatRoomFunctionalRecursive
improves upon that but may be harded to read).WDYT, @rkuhn?
The text was updated successfully, but these errors were encountered: