OrleansContrib / Orleans.StorageProvider.Redis

A Redis implementation of the Orleans Storage Provider model. Uses the Azure Redis Cache to persist grain states.
42 stars 20 forks source link

Orleans 2.0.0 #17

Open bboyle1234 opened 6 years ago

bboyle1234 commented 6 years ago

Here's a bunch of code I wrote: It's my best effort at setting up a Redis storage provider for Orleans 2.0.

https://gist.github.com/bboyle1234/38e766d2f5baa3d94ac10bce30c04c3f

What's the way forward with Orleans 2.0? (Asking so I know how to contribute)

galvesribeiro commented 6 years ago

@bboyle1234 you can make a PR updating this provider to use Orleans 2.0.

I don't know who was the original maintainer but, if it feels abandoned, let me know and I can get a revamp on the project to make it work on 2.0 properly.

martinothamar commented 6 years ago

Started using a Redis storage provider in an internal project now, it's up to date with 2.0.0-beta3, I'd be happy to update this repo to beta3 or rc-1 in a pull request unless @bboyle1234 has already started.

bboyle1234 commented 6 years ago

Hi @martinothamar @richorama @galvesribeiro , I've already made a pull request with code that performs multi-targeting. If the target framework is net461, it references Orleans 1.5.3, (basically no changes) and if it targets netstandard2.0 or netcoreapp2.0 then it references Orleans 2.0.0-beta3.

You can see a good example of the mult-framework targeting approach in theNewtonsoft.Json package. From what I've seen of Orleans packages so far, the trend seems to be a bit different and I'm not sure how the people in charge here would intend setting up their branches and packages to support continued upgrades for net461 (1.xxx) and net core (2.xxx), so I'm a bit stalled on contributing as I don't know how to move ahead.

galvesribeiro commented 6 years ago

Guys, there is no reason for multitargeting IMHO.

Keep a stable version updated to 1.5.3 and make a new one moving forward for 2.0.

2.0 has a different configuration mechanics and when we talk about storage providers, there are different interfaces in the new storage mode (e.g. IGrainStorage).

So I would recommend to not try to keep this multitargeting as Orleans itself will evolve only on 2.0 line and 1.5.3 will only keep severe build fixes.

I can help and port this provider to 2.0 myself if noone already did that.

martinothamar commented 6 years ago

I'm having at it now, will submit pr later. I'll do 2.0.0-beta3 first (need it at work), then rc1. I see the dev branch is pretty old, merge the 2.0.0 stuff into dev? Keep master in sync with 1.5.x?

bboyle1234 commented 6 years ago

Thank you @martinothamar, do you think we'll ever have a way to fix ETag functionality in the redis store? Any ideas?

martinothamar commented 6 years ago

Yeah I have a possible solution working, will submit a PR when we get 2.0.0-beta3 in

richorama commented 6 years ago

I would love to see a good solution to the ETag problem, especially if it could maintain backwards compatibility with existing state.

Going forward I accept @galvesribeiro advice. Orleans 2.0 has a number of differences. We should sunset the 1.x storage provider, and just focus on 2.0 (unless there are any objections?)

galvesribeiro commented 6 years ago

What is the etag problem? Doesn't Redis have ETag by itself and support conditional writes? If not, there are alternatives like the ones I did on AWS DynamoDb provider as it doesn't have ETag as well.

richorama commented 6 years ago

@galvesribeiro last time I checked it didn't have any Etag (or similar) support.

see issue #9

galvesribeiro commented 6 years ago

Ok... Did you guys checked that: https://github.com/StackExchange/StackExchange.Redis/blob/master/docs/Transactions.md#and-in-stackexchangeredis

It seems that even multiplexing connections (which I assume the provider is) you can kind of lock the transaction and put a constraint on its body which can evaluate a field which in this case, is the ETag.

Also, I noticed that the project still uses the old tooling and that is more one reason to hang a stable version now pre-2.0 and restructure the project solution upgrate to new .csproj format along with new Orleans 2.0 packages and its APIs. My VS can't even open the solution without migrate the projects :(

galvesribeiro commented 6 years ago

I could even suggest that that all the WRITE ops were made using Redis scripts (Lua scripts actually) just like I did for CosmosDB using JS procedures. That will ensure a db-level transaction guarantee.

martinothamar commented 6 years ago

I have a Lua-script for conditional write based on ETag, I can submit it later and we can all have a look.

martinothamar commented 6 years ago

The .csproj files are updated to the new format in the dev branch btw. We could branch the old stuff in master into a 1.5.x or legacy-branch and keep master in sync with the new stuff maybe.

galvesribeiro commented 6 years ago

Good @martinothamar

Also it would be good to follow the new naming convention for the packages/namespaces/extensions so it reduces friction from newcomers.

As I mentioned, if that is something you guys are busy I could do that on my free time at night.

jmvermeulen commented 6 years ago

I've run this provider successfully on rc-2, now with 2.0 there is a compile error.

 public RedisGrainStorage(
            string name, 
            RedisStorageOptions options, 
            SerializationManager serializationManager,
            IOptions<ClusterOptions> clusterOptions, 
            ILoggerFactory loggerFactory
        )
        {
          ...
            this._serviceId = clusterOptions.Value.ServiceId;
        }

The ClusterOptions.ServiceId has changed from Guid to string.