Make the GrainClient class non-static

[nehmebilal]

Connecting to multiple Orleans Silos from the same client is not possible because the Orleans client class is static and there can be only one of it in a process. Making the GrainClient non-static will help solving this issue.

Client code will create an instance of GrainClient instead of using the static GrainClient.Initialize as follows:

        IGrainClient orleansClient = new GrainClient(“DevTestClientConfiguration.xml”);
        IGrainFactory grainFactory = orleansClient.GrainFactory;
        IHello grain = grainFactory.GetGrain<IHello>(“0”);

---

@gabikliot wrote:
TODOs:

• Remove all static mutable fields from client runtime.
• Static Interners
• RuntimeClient.Current.SendRequest
• Static TraceLogger instances
• Static mutable fields inside TraceLogger
• Placement Singletons
• Serializer internal collections
• All statistics counters

[gabikliot]

This is a great initiative.
Lets try to make progress on that in steps. First step that I would recommend is to identify all places within the runtime that rely on the runtime to be singleton. This goes beyond just GrainClient class. And of course, we won't be able to remove singleton staticGrainClient until we remove all the internal runtime static singletons.

So the 1st one to look is RuntimeClient, which has a static RuntimeClient.Current of type IRuntimeClient.
This RuntimeClient.Current is what allowed us to have the same code in multiple places for silo and client runtime. One notable example is GrainReference.InvokeMethod_Impl calling RuntimeClient.Current.SendRequest. To fix that we would need to inject some way for all grain references to call SOMETHING.SendRequest, which will be different on the client and silo side.

So I suggest we focus on this place first. Once we solved this place, we can identify all other similar places and only after we changes those we can look at making a breaking API change to application code, which will be a result of making GrainClient class non-static.

[ReubenBond]

Early thinking on this, looking for feedback:

If we want to support multiple grain clients in a given process/AppDomain calling into different clusters, then each GrainReference should have a reference to the IRuntimeClient for that cluster. That's easy enough. Issues become more apparent when we talk about serializing/deserializing grain references. When a GrainReference is sent over the wire or to/from storage, we need some way to associate it with the correct cluster. This could be done either automatically or manually.

Fully automatic/transparent association would be ideal: grain references could be passed between multiple clusters & client, and stored & retrieved with no intervention by the user.

To illustrate how a fully automatic system could work @sergeybykov pointed out the extreme case where all of the information required to reconstruct the grain client is serialized along with each grain reference. Such a system would likely be too heavy in practice (grain references are supposed to be cheap).

Another option is to only support automatic association when communicating references within a given cluster. This should support the 99% use-case where there is only one cluster. To implement this we would automatically associate grain references during deserialization - the deserializer would need context about the IRuntimeClient performing the serialization. I think it's not a bad idea to pass a non-thread-local ISerializationContext/IDeserializationContext with an IRuntimeClient property to the serialize/deserialize methods (changing the method signature).

Under this option, we could adopt the following rules:

1. If a GrainReferences's IRuntimeClient is equal to the ISerializationContext's IRuntimeClient, then it is serialized as a "bound grain reference" and is automatically rebound to the IDeserializationContext's IRuntimeClient during deserialization.
2. Conversely, if a GrainReference's IRuntimeClient is not equal to the IRuntimeClient in the ISerializationContext, then the reference should be serialized as an "unbound grain reference" and not be automatically bound during deserialization. In this case, the user must intervene before attempting to call the grain reference.

If a call is made on an unbound grain reference, we will throw a strongly typed exception.

Because we do not always know which IRuntimeClient to associate with each reference (case 2) and since most serialization libraries don't allow injecting contextual info during deserialization (eg the IRuntimeClient), we should allow users to associate GrainReferences with clusters manually.

We could accomplish this using an API like so:

var client = new GrainClient(config);
client.Initialize();

var user = mySerializer.Deserialize<IUserGrain>(serializedReferenceFromStorage);
client.BindGrainReference(user); // bind the user grain reference to the specified client.

// Alternative API using an IAddressable extension method:
user.BindGrainReference(client);

Finally, the above system could be extended if each cluster had a unique id. In such a scenario, we could potentially create a map of id->client and use that map when deserializing each reference. When communicating references between grains/clients in the cluster they belong to, we could avoid serializing the cluster id and follow rule 1 above. Rule 2 would become the following:

1. If a GrainReference's IRuntimeClient is not equal to the IRuntimeClient in the ISerializationContext, then the reference should be serialized as a "foreign grain reference" and include the foreign cluster id. The reference should be automatically bound during deserialization if the cluster id is present in the IDeserializationContext's id->client mapping.

If we adopt this extension then we can adopt an API to support binding a GrainReference to the matching IRuntimeClient in a collection of clients:

var clients =
  new Dictionary<ClusterId, IRuntimeClient> { [clusterA] = clientA, [clusterB] = clientB, };

if (!grain.TryBindGrainReference(clients)) throw new Exception("!!!");

[sergeybykov]

Finally, the above system could be extended if each cluster had a unique id.

The scenario that makes me skeptical of the cluster ID direction is when the app persist a grain reference in storage, and reads it a month (or 12 months) later. The cluster ID would have to be an immutable variable in the deployment for this to work, and the cluster would have to be available at all times later.

This makes me think that a more pragmatic solution is the manual binding one. Less magic, no implicit long term commitments to config variables like cluster ID.

[ReubenBond]

Less magic, no implicit long term commitments to config variables like cluster ID.

I'm all for it, especially initially. It's just worth noting that we could extend the concept to support auto binding in a wider range of scenarios in future.

Side note: we can keep GrainReference deserialization backwards compatible with the current iteration by hijacking the byte which is written after the GrainId. Currently the least significant bit is being used to determine if the ref is a system target or not. We can piggy back on the other bits to either add a version field or just a simple flag.

[jdom]

👍 identical to how I envisioned this too, with the exception of the bind method name (Yeah, that's how identical to my thoughts it was). I did also consider a few similar things especially with regards to the multi cluster support. Agree that it's an improvement that could be done later, since we should support a way to (re) bind manually anyway.
Also there could be a way to optionally specify a iruntimeclient resolver instead of always throwing an exception with an unbounded reference. With that, if the user does not care about multi cluster support, he can fallback to a singleton (or custom logic too to determine the target cluster) and be back compat

[ReubenBond]

@gabikliot I want to ensure that you have visibility into this, since you may have feedback on the proposed strategy discussed above.

[gabikliot]

Sure, I will take a look.

[ReubenBond]

This issue is essentially resolved by #2822. There are still outstanding issues with regards to logging, metrics, and maybe some other classes being shared between instances.

Multiple clients can be used in a single AppDomain, clients can be created and used from within silos. Work is underway to give Silos the same treatment with a 'silo builder' and that will hopefully include fixes to make logging & metrics non-global. SiloBuilder design is being discussed in #2936

IMO, we can close this issue and if there are specific areas which still need to be addressed then we can open issues for those.