[FEATURE] Upgrade to .net 7? about orgnalr HOT 21 CLOSED

Thanks for asking! Yeah there's a bit more of a migration strategy than previous upgrades, and it seems that .netstandard support has been removed from the orleans dependencies we use. This may not be a huge issue, and we'll need to target different orleans packages (specifically Microsoft.Orleans.Sdk).

https://devblogs.microsoft.com/dotnet/whats-new-in-orleans-7/

Few more breaking changes so this would need to be a major release:
https://learn.microsoft.com/en-us/dotnet/orleans/whats-new-in-orleans

from orgnalr.

Yes; there are a ton of breaking changes. Frankly, it's why you don't have a PR waiting now from me :) We need a solution for a SignalR backplane on .net 7 and this is the most attractive library I've found. (I very much like that it doesn't use streams under the hood). I'm happy to do a ton of the heavy lifting. Ran into an issue getting the tests updated.

from orgnalr.

Awesome I'll have a crack at getting it all working together tonight, thank you! I haven't been following along with the breaking changes, but I'm glad they're happening, there's a bit of cruft in the core lib that was awkward to work around

from orgnalr.

G'day thanks for your help! I've merged it and deployed a 2.0.0-prerelease1 package. I'll be testing it on my projects tonight, but feel free to have a poke

from orgnalr.

from orgnalr.

Some pretty extensive changes - I haven't been able to successfully migrate an existing project yet - but I'll give it another crack tomorrow. At the very least I should be able to get it up and running on a new project

from orgnalr.

I got all the tests passing except the first one using the built in Orleans Test stuff they want you to use. I spent a bit on the failing test, tbh I'm not sure how that one worked to begin with. Could you take a crack at it?

from orgnalr.

We are using this 8 times in OrgnalR
https://learn.microsoft.com/en-us/dotnet/orleans/grains/external-tasks-and-grains

from orgnalr.

Looking at that it's not being used in Grain code. I went ahead and left it there instead of deleting it to get confirmation on it

from orgnalr.

Yeah to be honest I think we can remove those ConfigureAwaits - in the context of where OrgnalR will run the context will be the ThreadPool since .net core. Now we've dropped support for .net standard (and therefore .net framework) - no need to clutter the codebase.

I'll have a crack at the tests tonight

from orgnalr.

from orgnalr.

Awesome - Got that test working. It was mainly showing that getting all messages after a handle works as expected. Orleans (or at the very least, the test framework) used to give objects back with referential equality, however this seems to have changed

from orgnalr.

TLDR: I got it over the line after I wrote this message - so feel free to skip. The main takeaway is that serialization needs to be explicit, and we need a way of waiting for the client to be ready (still unsolved, currently just doing a Task.Delay(5000))

So good news, bad news! I eventually got it loading with an example app I'm working on. There was a race condition between setting up the OrgnalRHubLifetimeManager, and the IClusterClient. Basically we were trying to create a reference to a grain before it had made the connection.

This class in Orleans has a field which is set after construction:
https://github.com/dotnet/orleans/blob/f86e28cc31c2f78291dfa5a8e2e1fef05ef73515/src/Orleans.Core/Runtime/OutsideRuntimeClient.cs#L37

Adding a delay before trying anything works - but I hope to find a more elegant way of saying "Wait until ready". Got this PR which when running the example app, starts up properly (still can't serialize messages though):
#25

The other (workable) issue is that we were relying on implicit runtime serialization to serialize the messages that would go through signalr. Since Orleans now needs explicit serialization for messages - we'll need to do this. I am thinking of just sidestepping orleans serialization and sending all the messages as some format over the wire (JSON or some such).
It would be good if it is pluggable so the consuming application can decide how their messages are serialized, but this will need a bit of thought.

Right now that investigation branch above works only until you send a message. Then orleans tries to serialize the request object and hates it. Hopefully a strategy like the one outlined above gets us over the line

from orgnalr.

So I was able to update the example to have their messages serialize:
#26

I don't think we want to enforce that - it seems a bit strict. But it does work. Note you'll still need to apply the sleep from #25 to have it fully work

from orgnalr.

#28

I've done up something which works end to end - the only inconvenience is that all Hub message types need to be serializable with orleans.

What it does is before sending off to orleans, the backplane will serialize the messages to byte[]. It acheives this by using the Serializer which the Orleans runtime uses to serialize messages. This way we can piggy back off of the fact that the user will be using Orleans.

This does mean that you need to annotate your Hub message types with GenerateSerializer (see the example). I originally thought it too much of an inconvenience, however users of Orleans 7 need to do this pretty prolifically, so it shouldn't be that much of a burden.

from orgnalr.

Awesome - Got that test working. It was mainly showing that getting all messages after a handle works as expected. Orleans (or at the very least, the test framework) used to give objects back with referential equality, however this seems to have changed

https://github.com/LiamMorrow/OrgnalR/blob/master/src/OrgnalR.Backplane.GrainImplementations/RewindableMessageGrain.cs#L80

Creating a new message handle there means it's not possible for the referential equality check to pass because we create a new one. I don't understand how that ever worked.

from orgnalr.

#28

I've done up something which works end to end - the only inconvenience is that all Hub message types need to be serializable with orleans.

What it does is before sending off to orleans, the backplane will serialize the messages to byte[]. It acheives this by using the Serializer which the Orleans runtime uses to serialize messages. This way we can piggy back off of the fact that the user will be using Orleans.

This does mean that you need to annotate your Hub message types with GenerateSerializer (see the example). I originally thought it too much of an inconvenience, however users of Orleans 7 need to do this pretty prolifically, so it shouldn't be that much of a burden.

Yes, the Serialization stuff they've done is pretty intrusive. They even had to make a custom visual studio shortcut to set the indexes on the properties. I'm VERY happy we arrived at the conclusion that "all hub messages need to be serialized" instead of "it works until you send a message".

Microsoft knows it's a PITA. However, I think we'll all appreciate it when we need to add new properties to serialized types. https://learn.microsoft.com/en-us/dotnet/orleans/whats-new-in-orleans#serialization

from orgnalr.

Creating a new message handle there means it's not possible for the referential equality check to pass because we create a new one. I don't understand how that ever worked.

While we create a new handle, the message itself is (was) the same instance. We don't new up new instances of passed in messages, and the test was ensuring we get back the messages we sent - not the message handles

from orgnalr.

Yes, the Serialization stuff they've done is pretty intrusive. They even had to make a custom visual studio shortcut to set the indexes on the properties. I'm VERY happy we arrived at the conclusion that "all hub messages need to be serialized" instead of "it works until you send a message".

Microsoft knows it's a PITA. However, I think we'll all appreciate it when we need to add new properties to serialized types. learn.microsoft.com/en-us/dotnet/orleans/whats-new-in-orleans#serialization

Yeah honestly it's not a bad system - seems similar to Protobuf. The roslyn fix is definitely welcomed, and it is good it works with records.

But yeah! One more night of testing (if I manage to update my projects tonight I'll consider it a success). Then I'll look to release 2.0.0.

Do you have any objections to marking your Hub requests/responses with the orleans serialization annotations?

from orgnalr.

I was able to successfully upgrade the app, and it found a couple serializer issues. All fixed, and have release version 2.0.0.

Thank you for all your help @bbehrens ! Please reach out if you have any issues using it

from orgnalr.

I'll track the workaround for cluster client not being ready here:
#29

from orgnalr.