Enable grain methods with void return

[ReubenBond]

Internally, Orleans has the capability to support void-returning remote method calls. These methods are fire-and-forget and there is no natural way for the caller to determine whether or not the message was delivered and processed.

Currently, void methods are only enabled for IGrainObserver interfaces.

I propose we expose this functionality for all users. The alternative today is to call .Ignore() on a task a user doesn't care about, but this still incurs the performance penalty of scheduling a continuation.

Does anyone have objections?

[dVakulen]

Referencing some discussion from gitter on this topic - https://gitter.im/dotnet/orleans?at=56b074f6dc33b33c75494db0 -> https://gitter.im/dotnet/orleans?at=56b10170f98673e76f1a5b95 and relevant doc page with reasoning about design choices http://doc.akka.io/docs/akka/current/general/message-delivery-reliability.html

[dVakulen]

Also cost of continuation scheduling is negligible compared to the price of sending unneeded response message back to the caller.

[gabikliot]

Orleans in an RPC system. Rpc by definition is two way. Try suggesting to add a one way messaging to grpc for example. It's paradigm mismatch. Observers were a mistake in my view and should be discouraged instead of extending them.

[sergeybykov]

I can argue both sides here. :-)

On the one hand, I believe our async RPC model encourages writing safer code with proper error handling, introduces implicit natural backpressure, and eliminates the need for error prone correlation of one-way requests and responses. On the other hand, there are legitimate cases where unreliable one-way messages are all the app needs, and sending empty responses is wasteful. Coincidentally, I was in a discussion with Minecraft about one such design recently.

I think there are two loosely connected parts of the picture here.

1) On the caller's side, a void method call to a grain looks indistinguishable from making call to a Task-based method and Ignore'ing the returned Task, just shorter and cleaner.

2) On the grain side, a void method I believe is undesirable because it doesn't fit the async execution model, doesn't allow to make async calls from it, etc. And async void isn't a solution. It's a problem that I think we don't want to touch with a 6ft pole. It was meant for very specific use cases, and grain methods aren't even close to those.

I think the ideal case would be if we could allow void methods in grain interfaces, but then somehow make them Task methods on the grain class that implements it. Unfortunately, the basic interface implementation rules don't allow that.

I wonder if a reasonable compromise would be to keep such methods as normal Task-based in grain interfaces and classes, but provide a way to invoke them on the caller side in a way that would indicate that: a) I don't care about the result; and b) send the request as a one-way method. Something like an "extension method" but not for a type, but for a Task-based grain interface method. I think that this would fulfill the requirements of this issue without messing up the grain execution model.

[ReubenBond]

@gabikliot I think we should support fire-and-forget calls, but maybe void is the wrong way to do so - for the reasons @sergeybykov described.

So let's change the goal from void to fire-and-forget. Here're two options for supporting that:

• Add a [OneWay] or [Notification] attribute to grain interface methods so calls to that method never receive a response from the remote caller. The caller would receive a completed task from the generated GrainReference and the message would be sent OneWay at the codegen level (which is where we currently encode that flag)
• Allow some kind of customization at runtime to allow callers to indicate whether or not they care about responses for individual method calls. this might be via some using statement surrounding the call or some extension method before the call is made (implementing this safely without allocs may be tricky)

I prefer the first option

[sergeybykov]

The problem I have with attributes on grain interfaces is that it's the caller's decision whether or not they are interested in the response, not of the implementer of the grain interface. Ideally, this functionality shouldn't require any change of the grain interface/class code, so that the caller could make a one-way call to any grain method without the need to add attributes to it first.

If we could support something like the following, I think it could be acceptable.

public interface IFoo : IGrainWithGuidKey
{
    Task Bar();
}

var g = GrainFactory.GetGrain<IFoo>(key).WithOneWay();
g.BarVoid()

where BarVoid() calls an alternative invoker (on the caller's side) for Bar that makes it a one-way call and returns a void.

However, I don't quite see how to make WithOneWay return an extended (codegen'ed?) version of IFoo with a void BarVoid() added.

All names are, of course, for illustration purposes only.

[ReubenBond]

I find WithOneWay() scary, though. It changes the meaning of the entire GrainReference forever, it's all-or-nothing and could lead to bugs where you pass that ref to other callers.

What if we added a new Task-like type which then supports async/await, AsyncVoid or VoidTask or something? I believe that whether or not it's important to await a given method invocation should usually be encoded into the method declaration.

[ReubenBond]

We could support this via return types as well as some kind of one-shot method for making a message one-way

[gabikliot]

Would it be correct to say that all legit cases for one way messaging fall under streaming? In which case we could/should encourage to use streaming and not support one way messaging?

[ReubenBond]

@gabikliot no, I don't think so. I believe it would be legitimate for users to want one-way messaging. For example, in the transactions code, grains have an Abort method which is really only a notification - the transaction manager does not need to wait on it and waiting on it really doesn't signify much. The message doesn't affect correctness of the algorithm: it's an optimization which allows grains to proceed with other requests more quickly. Why would I want to use a stream for that?

[gabikliot]

Yes, that is not for streaming. But that is also arguably not a good scenario for one way. It's not correct to say this msg is not important. Otherwise it's redundant.

Look, at the end , it's the same argument all over again: risk vs. cisc, monolithic vs micro kernel, small flexiable core abstraction vs. rich multi feature abstraction, Corba vs. gRPC,..... You name it. Keep adding more tweaks and bells and whistles and you make the framework heavier, more complicated, harder to drive, cisc. At this point I am still in favor of smaller abstraction, but I understand that you guys are under pressure to adhere to what customers ask, which is features. Each individual customer can't think about the whole runtime direction or high level principle. It's your role.

[ReubenBond]

I'm also in favor of supporting grain methods which return multiple values (eg, IObservable). 0, 1, many seems right to me. In the transactions case, the abort message is important for responsiveness but not correctness (the grain will recover). It's also important that the caller (transaction manager) isn't blocked waiting for a response it doesn't care about. So we can eliminate the cost of that useless response message by not having one.

[gabikliot]

Yes, any extra feature is useful. I was not saying that in a general case one way messaging is never useful. Sometimes it is. We COULD optimize. The question is if we should. That is my argument about " more features is not always better". It's always a trade-off. And I am not saying we should object any new feature.

But for one way messaging I think the benefit (number if cases when it's beneficial) is small and the benefit of having one smaller abstraction out weights it. This extra feature will confuse the general users, who don't built transaction managers. For distributed apps it's mostly anti feature. And notifications/observations are better use streaming.

[sergeybykov]

I share Gabi's "less is more" concern in general. Though I do see a legitimate case, a minority one, in between RPC and streaming. That's why I'm looking at this potential feature more as a "one-way Ignore." Ignore is a rather rarely used but still important feature. If we can come up with an API solution to invoke normal grain methods returning Tasks in way that would indicate "send a one-way message and don't bring the result back" without confusing or cluttering the normal use case, then IMO we can walk the fine line here.

I don't quite see the API yet. Ignore is called too late. WithOneWay has issues that Reuben outlined. Changing return type from Task to something else I think would pollute the API surface.

[dVakulen]

The only API that comes in mind looks like this:

public interface IFoo : IGrainWithGuidKey
{
    Task Bar();
}

var g = GrainFactory.GetGrain<IFoo>(key);

g.ExecuteOneWay((IFoo grain) => grain.Bar());

Where "ExecuteOneWay" will provide to the lambda copied from original grain reference which will pass "InvokeMethodOptions.OneWay" parameter to the "InvokeMethodAsync" method.

[jdom]

I agree, and this ExecuteOneWay extension method can even be in a namespace that is not commonly used, if we are worried that the average users might get confused.

[ashkan-saeedi-mazdeh]

@ReubenBond Thanks for bringing it up again! :)

@sergeybykov @gabikliot To me this is one of the building-block features which is not one of the cases that Gabi mentioned, As a game developer I probably understand the minecraft guys concerns. They want to load huge voxel worlds and send information of each chunk to a player which is in the area of interest and the data for each player need to be packed from the world based on her AOI (area of interest), When plaeyrs/objects update the world and are not interested in getting any world updates back, the two way messages are a huge waste and using streams for all one way messages in such system would make Orleans a messaging system.

All this said as a game developer I'm used to one way RPCs in multiplayer games so RPC to me was always one way and the 2 way ones are a convenience. I understand the regular Nodejs convert doesn't need this or does not care about these.

But features like placement strategy, choice of one-way vs two-way messages can be deal breakers for such a framework which is still pretty low level, no matter how hard we try to make it look like something highlevel using ordinary classes and interfaces.

API-wise I think @sergeybykov 's one wasn't as scary as Reuben thought , @dVakulen 's suggestions means much more allocations so something based on codegen with less allocations is more important

I mean less allocations is the + side of @sergeybykov 's suggestion but instead of doing it in the place that we get the grain reference, Codegen can generate both one way and two way versions of a method which is marked with [NeedOneWayVersion]

And then we have Bar and BarOneWay which both can be called. No lambdas and no scary reference semantics.

[dVakulen]

Few notes about 'allocation-heaviness' of https://github.com/dotnet/orleans/issues/2893#issuecomment-291806366 :

• Copied grain reference as well as passed lambda can be easily cached in case of need.
• This objects will have very short life time and usually will be collected in gen 0, so their overhead will be, in most cases, negligible.

[dVakulen]

@ashkan-saeedi-mazdeh Can you please provide some vision onto how users is expected to call that code-generated (in implementation) one way version of method?

[ashkan-saeedi-mazdeh]

@dVakulen I agree that allocations will not be much but what about something like this

public interface IPlayer : IGrainWithGuidKey
{
[EnableOneWay]
Task ChangePosition(Vector3 pos);
}

//User code
var player = GrainFactory.GetGrain<IPlayer>(p1);
player.ChangePosition(Vector3.Zero).Ignore(); //Two-way
player.ChangePositionOneWay(Vector3.Zero); //One-way

[dVakulen]

@ashkan-saeedi-mazdeh IPlayer interface does not contain definition for ChangePositionOneWay method. How do you expect this to work? This approach mixes complications of https://github.com/dotnet/orleans/issues/2893#issuecomment-290870736 with bad sides of adding of attributes to grain interface methods.

[ashkan-saeedi-mazdeh]

@dVakulen This is very different, sorry for not being clear. Here the callee doesn't decide anything about how one caller chooses to call ChangePosition. The attribute will cause code-gen to generate two methods (ChangePosition and ChangePositionOneWay) and the caller will decide if he/she wants to call the version which returns a Task and is two way or the version which returns a task but the task gets done as soon as the message is sent inside the generated method and doesn't wait for the response and no response is sent back at all either.

[dVakulen]

Attribute in interface definition means that in operation contract needs to be expicit assumption about which methods will require one-way version. The ChangePositionOneWay will be generated in interface implementation about which caller does not know. How can he call that method?

[ashkan-saeedi-mazdeh]

Hmm firstly the method should be added to the interface as well and secondly it can be added automatically for all methods returning Task and not Task

I think these kind of assumption in the interface is much more ok than definitely deciding if the method has to be one way or not,

Your way seems cleaner now however and probably worth the price maybe, I am not fully sure.

In most cases implementer of the contract and impl is the same person so at least that is not a huge concern but the more I think about it maybe something better should be found.

I'm afraid generating one-way versions for all methods would waste memory and make the size of the app much bigger as well.

The way that in some RPC systems you define how a method should be executed should be called like with reliable/unreliable communication and other properties is similar to what you suggested as well. Something like

RPCCaller.Call(RPCMethod(),CommunicationType.Reliable,CommunicationOptions.BufferRPC); //to execute this on later instances of a program connecting to us as well

I wonder what @ReubenBond and @sergeybykov think

Another option like Sergey's scary version is for the GrainRef to have a SetCallProperties method

var player = GrainFactory.GetGrain<IPlayer>(p1);
player.SetCallProperties(CallOptions.OneWay);
//Now calls will be one way

Is it more performant, it is less explicit and can be scary again however.

[ashkan-saeedi-mazdeh]

I don't know how much hard implementation-wise it is but another option is the ability to define a last argument in interface methods which takes an additional parameter of some type like

[flags]
enum CallingOptions
None=0,
Oneway=1,
...
}

Which is not sent to the target and only sets the properties of the call. In the other side it could have a default value.

However this has disadvantages as well. A wrapper method more and more seems the only option

[ashkan-saeedi-mazdeh]

Based on my discussion on gitter with @ReubenBond and @galvesribeiro and the more I thought about it, The requirement of caller control regarding a message being one-way or two-way is a bit odd and not much logical.

The description and reasoning is that a message should be designed for being one-way specifically. In a more broad view, For example network libraries for games allow you to call each RPC with different QOS channels which might be reliable/unreliable, ordered/unordered but practically in my more than 5 years experience of multiplayer game development you can not design an RPC which is sufficiently complex and can work without assuming its channel's quality of service. If a message is sent without order, you have to handle order and repeated message issues, If it is sent on a reliable ordered channel then you should deal with latency of arrival much more than an unreliable channel.

Here the case is the same, A message when implemented should know if it will be called in a one-way or two-way manner to be implemented correctly and sometimes safely as well. The question now is that why we should have it in the interface? Because on the interface generally each message will be defined completely, regarding what it should do and what types it has so why not also say how it will be called when each implementation have to assume this at the end and caller's most amount of control would be to use the one-way impl or the two-way.

Also I've never seen an RPC (at least in games and simulations) which I write and want to call with multiple channels of quality. This is obvious enough now that in Unity's new API, actually you choose the channel when you define the RPC and not when calling it. Your message's quality of sending and the guarantees of the sending channel are usually directly related to the functionality of the RPC so if it a high rate message which stale values are useless, it is unreliable and unordered. If it is a trading message then reliable ordered is the natural choice or reliable depending on the fact that you do multiple trades within few seconds or not.

Here the messages which are two-way or one-way are used for very different cases so chances are near zero (at least to me and a few others) that you want to call a method in both ways assuming you can implement it in that way without adding much boiler plate,

You only will define a method as two-way if you care about its return value or failure state or from the other way around you only define it as one way if you don't care about them. Let's don't think about optimizations which you have to do sometimes because you can not afford two-way messages. In those cases you might want to send an error message back with number of failed attempts if they go beyond a threshold. but that doesn't change the matter at all either.

A game example would be.

• We have a set of world chunk grains and a world grain managing them.
• each chunk can contain X player grains
• Each player grain receives messages/RPCs from client actions
• Each player grain sends all world changes of the last network frame based on its area of interest (the players and the world parts which the player is interested in knowing about and is updating on the client) to the client.
• Players can move between world chunks and say each world chunk is a silo -Players near borders of chunks are replicated in the neighbor world chunks as well.

Now for these two work, For most of this actually the routing system doesn't need two way messages for the time that a player is updating itself because even if it failed, the client can not do much and game engine networking systems usually just care to tell you if the message is received or guarantee it and not complete and correct execution on the server. This is because you can not do much either even if you know. The server should detect that and do something about it so sending anything back is useless. And you almost never want to receive an answer back for some message sometimes Either you are doing synchronizations of important events or trading which the game can not continue without them and want an answer or you are sending input and you don't want anything back since it is not useful.

For server to server (silo to silo) replication the case is different probably but you don't want two-way messages always and it might be interesting to know general persisting failures which probably are detected in other ways.

Now this is a game only example but in general the feature is only interesting in high rate messaging apps which can afford to lose the result of some operations sometimes or have to do that because of perf realities. Games and simulations are the most important ones coming to mind. In some cases you want aggregate errors for correcting things but that is a specific two-way message and not related to calling a message, sometimes in a one-way manner and sometimes not.

Assuming this the design would be really easy using marking the event by attributes or using specific return types as well.

[sergeybykov]

I see two cases being discussed here:

1. The grain method is intended to be used only in the one-way fashion
2. The grain method is generally meant to be used as a normal RPC, but sometimes, for optimization purposes, it makes sense to invoke is as one-way.

Case 1 is already covered by observers. For better or worse, the feature already exists, and I don't see a reason to duplicate it in a slightly different form.

For case 2, @dVakulen's proposal with an extension method so far looks the most elegant to me.

g.ExecuteOneWay((IFoo grain) => grain.Bar());

[ashkan-saeedi-mazdeh]

@sergeybykov I'm writing this here so it won't get lost in the conversation on gitter.

I think observers aren't good at all and not good specially if you have a lot of one way messages since they are not a part of the natural model, you have to handle subscriptions and also the receiving method can not be async. If I have to use something already built-in, It would be fire-and-forget SMS.

If that is your conclusion I would choose @dVakulen 's approach over not implementing it. His approach doesn't feel right to me (for the reasons above) and has a bit of allocation as well, but has the advantage of being pretty obvious and is loud about it being one way when you read the code, so I understand the appeal.

I could not convince you that the case for calling a method as one-way sometimes is not a real use-case (any usecases @dVakulen ?)

I wonder what @ReubenBond thinks but in any case I would like this to be in even if it is using the ExecuteOneWay approach. Observers are something which we should replace with this and fire-and-forget SMS.

Most people who used them had issues and talked on gitter and when they moved to streams they lived happily for ever after that :)

[ReubenBond]

@ashkan-saeedi-mazdeh my preference is still either a special return type or a [OneWay] attribute on a Task method.

Most people who used them had issues and talked on gitter and when they moved to streams they lived happily for ever after that :)

This seems to be the case. Personally, I was fine with observers, but maybe that's because of how I used them, resubscribing on a timer, holding the references so they didn't get collected, and even then they were just an optimization over polling (cache of notifications kept on per-user grain).

[ashkan-saeedi-mazdeh]

@ReubenBond said

This seems to be the case. Personally, I was fine with observers, but maybe that's because of how I used them, resubscribing on a timer, holding the references so they didn't get collected, and even then they were just an optimization over polling (cache of notifications kept on per-user grain).

Yes in specific cases they are good enough.

I highly agree with your [OneWay] or specific return type as well and it is the most natural way to me.

@sergeybykov Can you let us know why do you think the other is more elegant other than the fact that it makes it too obvious that it is one-way?

To me this is an advanced feature only used in certain cases maybe to build even frameworks inside Orleans for other developers in the same team and naming convention would resolve the issue of that OneWay is not obvious (I guess return type is obvious enough too).

Also the advantage of this would be that it would look more natural and a part of the model, it doesn't allocate anything in addition to what is already being allocated (even if that is just a cached lambda) and it is aligned with the real use-cases of the feature.

I am really curious to know any method which you want to take the pain of implementing in a way that is valid for both one-way and two-way calls and where and when do you want to call it one-way/two-way. This is almost always something chosen by the functionality of the operation and the context is defined there which shows if the method is one-way and we don't care about the result (or want a batched result of many sends after some time, say for aggregations and ...) or the method is something which should return a response and is two-way.

I get beauty of having the two-way request-response method but we are not supporting the other case other than by hacks using streams/observers which are for sending data/notifications to multiple subscribers. I guess from the day one this was a valid message type to have for grains. The only reason this is not added probably was for not having internal use-cases or for not making the framework complex I guess.

Please rethink about it and think about real use-cases of it. The freedom off calling any method one-way is againstt the programming model I guess since something defined to return a response should return one and the response should be taken care of. We should not allow people to skip it and shoot themselves in the foot. But if something is designed to not return a response directly, then it can be used in that way and adding support for it as a grain method makes cases like my game scenario above possible in a Orleans-native way :)

Hacking it with fire-and-forget SMS has other issues other than the verbose code like optimizations which might be done on SMS which aren't desired in the case of each stream having one receiver and one sender used for normal message passing.

@dVakulen has the disadvantage that anyone can think ok for optimization let's not receive responses of this method back. I can shoot myself in the foot with this thought since the method implementer expected a different thing, She might have thought that if I throw an exception then my caller will wait a bit since I can not access the DB and then will recall using exponential back-off or whatever.

In the case of specific return type the implementer is telling the caller that this method will not return you a response and knows that you'll call it no matter what happens in the execution of the method (probably these are all high frequency calls or calls in too many grains). In this way the caller can not cause bugs or other system-wide issues by not calling a method in a unexpected way. This is a very good thing to me since I am sure for one-way and two-way I would implement simple methods differently.

public Task<MoveResult> SetPosition(Vector3 position)
{
State.Position = position;
if(position.IsInAnotherChunk)
return Task.FromResult(MoveResult.ChangeChunk);
else
return Task.FromResult(MoveResult.ReaminInSameChunk);
}

I expect the caller to care that if the object is changing zone, tell the client to connect to the other chunk or for example know that probably it should batch move commands till the chunk change is complete with all entity data serialized and sent to the other zone.

In the one way case I don't return anything back and am handling the batching of move commands in a different way.

I don't want any of my team members to call this one-way in the name of optimizations.

Please bring up a use-case which shows the other way around.

[sergeybykov]

Observers are something which we should replace with this and fire-and-forget SMS. Most people who used them had issues and talked on gitter and when they moved to streams they lived happily for ever after that :)

There seems to be a confusion here between observers on the client and observer interfaces implemented by grain classes. The former has all the issues, but they are caused by the fact that clients are inherently unreliable, and really have noting to do with observers per se. The latter case works as reliably as normal grain calls - the target grain will get activated by an observer call if it's not in memory, just like with normal calls.

The advantages of #2993 over observers I see are a) such grain methods run as normal async methods, and hence can use await, etc., and b) they can be marked with the attribute on a method by method basis without an additional interface, which simplifies the usage.

The advantages of @dVakulen's ExecuteOneWay proposal over observers are the same. In addition, unlike #2993, it allows caller to invoke any grain method without the need for the grain implementer to change the interface.

I can see the argument that for serious performance optimizations marking methods explicitly by the implementer of the grain interface/class is probably not unusual to expect.

[sergeybykov]

After thinking some more about it. I don't see any harm in #2993 (with the caveat that I think we shouldn't allow it for methods returning a Task<T>). This will not clutter the API surface. It will give full control to the grain interface/class implementer with no additional cognitive load on the caller. I see a potential corner case here of caller being surprised not getting an error back even when an invalid combination of arguments is passed. But it doesn't worry me because it's the implementer's decision, just like with any method.

Neither do I see harm with ExecuteOneWay, if it's not in the main namespace, and hence doesn't pollute the mainstream API surface and requires some discovery effort. Seems complimentary to #2993, as it gives full control explicitly to the caller.

[ashkan-saeedi-mazdeh]

I think @ReubenBond 's suggestion of specific return type is the best then , something like IOneWay or whatever instead of Task, IOneyWay of course should be a sub-class of Task then to allow async methods.

[ReubenBond]

@ashkan-saeedi-mazdeh we can't have that yet, though - we need C# 7 to support custom async types (C# 6 has support for custom awaitables, but not generalized async/await on the method side)

[ashkan-saeedi-mazdeh]

@ReubenBond What about Task<OneWay> ?

[ReubenBond]

The main downside to Task<OneWay> is that the user has to return a OneWay instance. An AsyncMethodBuilder/custom type would allow us to avoid all of that, as far as I understand it.

[ashkan-saeedi-mazdeh]

@ReubenBond Yup you are right man. Do the thing you were doing.

Also @jdom 's optimization suggestion to allow void methods was interesting but not that much needed urgently.

[sergeybykov]

Resolved via #2993.