Implement POCO Grains

[ReubenBond]

Currently, all grain implementations must derive from Grain.
This issue is about lifting that requirement.

Motivation

Why remove Grain as a required base class? The primary reason, as I see it, is not practical at all. It's simply about removing what some may perceive as magic. When developers suspect magic is involved, it tends to cloud/inhibit reasoning abilities. Grains are not at all magic: they are simply implementations of some user-defined communication interfaces. The Grain base class is very thin: it largely just provides helpers to access other facilities (timers, streams, grain factory, identities, etc).

There are some practical reasons. For example, .NET classes can only have a single base class, which means that business logic which is Orleans-independent cannot be implemented as a grain by simply sub-classing and adding (eg) an interface which signifies that a class is a grain.

Progress so far

In #6585, we re-oriented the runtime around IGrainContext, which is a type the runtime uses to represent a grain activation. It's the IGrainContext which holds a reference to your Grain classes and which is responsible for all interactions with your grain implementation (activation, deactivation, message scheduling).

In #6446, @KevinCathcart implemented a proof of concept for how POCO grains could work - that lays a lot of groundwork and provides philosophy/guidance for how this should work as well as the potential issues which may require API changes.

There is a work-in-progress implementation: ReubenBond@2d09b70 (currently found in this branch, which also demonstrates "RPC Services" - multi-threaded/reentrant stateless services: https://github.com/ReubenBond/orleans/commits/feature/rpc-services)

Remaining work / discussion

Rather little, it's mostly about decision making and discussion. What remains is to remove some checks and casts to Grain and re-orient some remaining types (eg, IGrainRuntime, which exists primarily for unit testing) around IGrainContext instead of Grain.

AsReference<T>()

We also need some way to continue being able to support calling this.AsReference() and this.AsReference<T>() from a grain - extension methods which lets a developer get a reference to the grain which called the method. The methods currently rely on the caller deriving from Grain so that it can access the corresponding IGrainContext (Grain has an internal Data property which points to the context). If Grain is no longer required, we need some other way to enable this method. One solution is to allow developers to implement a new interface IGrainBase which has a single property IGrainContext Context { get; }. See this comment for more context: #6446 (comment)

API / ergonomics concerns

Should we require that developers signify that a type is a grain implementation somehow, or should we use logic that "any non-abstract class which implements the IGrain marker interface (no methods) is a grain implementation"?
Alternatives (among others) could be:

• Grain classes must inherit some IGrainBase marker interface - noting that IGrain is already used today for grain interfaces
• Grain classes (or some base class/interface) must have a marker attribute

The WIP branch linked above just uses the IGrain requirement. It only supports .AsReference<T>() if the grain happens to implement IGrainBase.

Example API

With IGrainBase to support .AsReference() (via the GrainContext property), OnActivateAsync, OnDeactivateAsync:

public interface IGrainBase
{
    IGrainContext GrainContext { get; }
    ValueTask OnActivateAsync(CancellationToken token) => default;
    ValueTask OnDeactivateAsync(DeactivationReason reason, CancellationToken token) => default;
}

public class PingGrain : IGrainBase, IPingGrain
{
    private IPingGrain _self;

    public PingGrain(IGrainContext context)
    {
        GrainContext = context;
    }

    public IGrainContext GrainContext { get; set; }

    public ValueTask OnActivateAsync(CancellationToken cancellationToken)
    {
        _self = this.AsReference<IPingGrain>();
        return default;
    }

    public ValueTask Run() => default;

    public ValueTask PingPongInterleave(IPingGrain other, int count)
    {
        if (count == 0) return default;
        return other.PingPongInterleave(_self, count - 1);
    }
}

Feedback and discussion appreciated

[nkosi23]

My first reaction was that this change would be great, mainly because it will make it easier for people like us using Domain Driven Design to turn their core domain model entities into grains as simply as by implementing Grain interfaces. The requirement to inherit from the Grain base class makes it impossible right now, so there is a need to introduce an additional Grain layer in the project structure, wrapping and seemingly duplicating the core domain model.

However, thinking about it deeply, I am not sure this would simplify the life of developers that much in practice because let's remember that Grain methods must be async - and therefore return Tasks. Therefore if one wants to use his core domain model as grains, he will need to extend the interface of the domain entities with additional async methods essentially wrapping the core methods (returning non-async values). So there'd be the same code duplication, but the class would now look more messy and arguably have too much responsibility.

Now If the programmer decides to treat grains as first-class citizens by making all his domain model entities expose async interfaces by default, then inheriting from a Grain base class is not a big deal since his design his already influenced by Orleans anyway.

My point is that the quality-of-life gains may be negligible in practice. And the quality of life may even be degraded if the Grain base class is removed and the developer now needs to manually do plumbing that used to be done automatically for him in the base class. With this regard, removing the magic perception should not be done at the cost of degrading quality of life (and for the records, I am among those seeing Orleans as magic 😄 ).

I therefore tend to think that it may actually be a good thing to have this base class, to facilitate quality of life conveniences, and to encourage developers to follow the single responsibility principle. With this regard, to me the fact that Grains must derive from the Grain base class actually is a reminder that Grains "are implementations of some user-defined communication interfaces". As a magic practioner lifting that requirement would tend to make me think that Orleans is some solid white magic stuff being able to send any arbitrary unserializable class over the wire.

[ReubenBond]

Thanks for the thoughtful response, @nkosi23.

I also believe that he Grain base class should remain: it's very convenient.

For serialization, Orleans currently now requires [GenerateSerializer] to be added to any serializable class, with [Id(int)] on each field/property - so hopefully that reminds people that it's not performing magic.

My general take is that helpers (like Grain) are good in practice, but they should only be helpers and developers should be free to build them up from scratch if they like. So assuming we add support for POCO grains, we shouldn't change our guidance

EDIT: I accidentally closed the issue - typing on mobile

[SebastianStehle]

I see 2 more advantages:

Less coupling

I think a lot of behavior is too coupled to the core at the moment and it would be easier to keep it outside.
For example I think that state management should not be in the core and it is very easy to implement it outside of the core. It is weird that the InconsistentStateException cannot be handled outside of this class:

orleans/src/Orleans.Runtime/Core/InsideRuntimeClient.cs

Line 311 in 78b828a

if (invocationException is InconsistentStateException ise && ise.IsSourceActivation)

This does not only improve the architecture but could also motivate developers to build more extension and pluggable behavior for grains.

I have 2 examples:

1. A grain limiter that ensures that old grains get removed: https://github.com/Squidex/squidex/blob/087f6d1a09ed4b7a9381d1f5217750f294cf87dd/backend/src/Squidex.Infrastructure/Orleans/ActivationLimit.cs

2. Custom storage solution: https://github.com/Squidex/squidex/blob/087f6d1a09ed4b7a9381d1f5217750f294cf87dd/backend/src/Squidex.Infrastructure/Orleans/GrainState.cs

Testability

I thin grains are too hard to test at the moment. You have to provide several mocks with unclear purpose and some methods like DeactivateOnDelay just throw exceptions in tests. It would be easier to just inject a few services and mock them when needed.

[JorgeCandeias]

TLDR1; Let us have:

public interface IGrain
{
    IGrainContext Context { get; }
}

And we can do:

public class MyOwnGrainBase: IGrain
(
    public MyOwnGrainBase(IGrainContext context)
    {
        Context = context;
    }
)

TLDR2; Or even better, just let us have:

public class MyOwnGrainBase
{
    [GrainContext]
    public IGrainContext Context { get; set; }
}

Time for some devil's advocacy.

I think the concept of "POCO grains" itself can lead to very chatty designs, when developers just take Orleans for a caching framework and nothing else. I've seen this manifest with Akka as well, and you end up with designs that are no different from a vanilla aspnet application with code and data still separate and moving through the network all the time. While these designs work, they miss the point and benefit of using actor frameworks, as we can achieve the same effect with Redis with less code. In my opinion, all this is an effect of marketing rather than technology, and I tend to push back against using actor frameworks just for POCO purposes, stressing that the benefit comes from having data and behavior together in the same place, one analogy being the benefits of stored procedures in databases to minimize network chatter and latency.

Devil's advocacy done now, I believe, in general, design changes that align it closer to aspnet web api standards can make Orleans easier to learn for dotnet developers, as I suspect many of them will already have been exposed by aspnet by the time they learn about Orleans.

The aspnet web api framework is reliant on attributes and other declarative programming artifacts and this appears to be well-accepted, as far as I can tell, provided there's no hidden conventions to confuse and surprise people. Like Grain, the ControllerBase class also acts as a giant helper to generate results and expose some async local services and assorted factories, just so we don't have to pull them in ourselves. They inject these services via an activator scheme upon execution, and based on injection attributes, but also allow unit tests to set or mock them at will by leaving all properties as public. I think these simple changes alone would help make the Grain base class more testable, as features such as streams are a hassle to test without a test cluster.

Following this train of thought, Blazor is another example where the team has embraced property based injection, now not only for the framework itself but for the developer as well, using the Inject attribute. I don't think Orleans needs this but that approach is getting pushed now on a flagship framework. And what can be injected, can be extracted as well.

With all the above in mind, I can see an approach where developers can just mark some property with a [GrainContext] attribute and let Orleans both inject and extract the context from this as required by its own operations. Same goes for other features as needed. Unit tests can then mock this as appropriate or ignore it if not required.

[ReubenBond]

@JorgeCandeias wrote

TLDR1; Let us have:

public interface IGrain
{
    IGrainContext Context { get; }
}

I considered this, but it's a little troublesome, which is why I proposed the optional IGrainBase. The trouble is that IGrain is what all grain interfaces/classes derive from, which means that any generated GrainReference must also have an IGrainContext, but the context is not (and shouldn't be) available to the caller.

Using attributes, such as [GrainContext] would mean that reflection is needed by the runtime in order to access the property for use in cases such as this.AsReference<T>().

I prefer we avoid magic property injection. One of the purposes of POCO grains is to have something which does not rely on magic. The Grain base class currently accesses the IGrainContext in its constructor by accessing the internal RuntimeContext.Current property which we set before construction. We could potentially expose that, but I prefer direct injection (no magic) for classes which don't inherit from Grain.

POCO grains aren't a convenience feature. Grain exists (and will remain) for convenience.

[JorgeCandeias]

The Grain base class currently accesses the IGrainContext in its constructor by accessing the internal RuntimeContext.Current property which we set before construction. We could potentially expose that, but I prefer direct injection (no magic) for classes which don't inherit from Grain.

If zero magic is the objective then how about accepting a context object in cases such as this.AsReference<T>() and make it this.AsReference<T>(IGrainContext context)? In this case, the grain class would not need to inherit from any Orleans interface to begin with, however, the occasional use of the context would force explicit code.

public class MyGrain: ISomeUserInterface, ISomeInternalUserInterface
{
    private readonly IGrainContext _context;

    public MyGrain(IGrainContext context)
    {
        _context = context;
    }

    public Task SomeWorkAsync()
    {
        var myself = this.AsReference<ISomeInternalUserInterface>(_context);

        return Task.CompleteTask;
    }
}

This gives us redundant code and no convenience, but also gives us zero magic, and unit test friendlyness.

The above said, I don't know how would Orleans figure out that this class is supposed to be a grain, without the use of, at the very least, some magic [Grain] attribute on the class.

I don't see any issue with having an accessible GrainContext.Current async local property available. This is the same approach as HttpContext.Current in aspnet and RequestContext in Orleans itself. I suppose Orleans would have to keep setting it before each turn executes as async locals will get lost but that's an implementation detail rather than an api concern.

[ReubenBond]

I think that's a viable alternative, @JorgeCandeias. or alternatively, developers can use _context.GrainReference.Cast<T>(). It's an API break and it also means that you cannot pass this to a grain interface call if you and have it always work

[SebastianStehle]

I think

this.AsReference<ISomeInternalUserInterface>(_context);

is strange for tests. How do you mock it? Not very obvious in contrast to something like

_context.GrainReference.Cast<T>()

[ReubenBond]

How about the following, for the issue of mapping from a grain instance back to its IGrainContext for POCO grains:

• We allow grains to implement IGrainBase which has lifecycle methods (activate, deactivate) as well as an IGrainContext field. It is entirely optional.
• Developers can inject IGrainContext and use context.GrainReference.Cast<T>() or they can fetch their own reference from the IGrainFactory by passing their own context.GrainId as a parameter.
• We enhance error messages to inform users of their options when (eg) they call this.GetGrainId() / this.GetPrimaryKey() / etc. This wont prevent mistakes, but it can help developers resolve them, at least. There is a core issue here: the same interface is used for a grain reference and a grain implementation and developers deal with those interfaces when writing code like .GetPrimaryKey() - we cannot statically prevent this if we support calling those methods. It's a balance between ergonomics/productivity and statically verifiable correctness.

[ReubenBond]

I've opened a PR for a work-in-progress implementation of "POCO" grains at #7360 - please let us know what you think.