Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Orleans 2.0.0 #17

Open
bboyle1234 opened this issue Mar 2, 2018 · 16 comments
Open

Orleans 2.0.0 #17

bboyle1234 opened this issue Mar 2, 2018 · 16 comments

Comments

@bboyle1234
Copy link

bboyle1234 commented Mar 2, 2018

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
Copy link
Member

@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
Copy link
Contributor

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
Copy link
Author

bboyle1234 commented Mar 10, 2018

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
Copy link
Member

galvesribeiro commented Mar 10, 2018

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
Copy link
Contributor

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
Copy link
Author

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

@martinothamar
Copy link
Contributor

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

richorama added a commit that referenced this issue Mar 13, 2018
@richorama
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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

see issue #9

@galvesribeiro
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor

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

@martinothamar
Copy link
Contributor

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
Copy link
Member

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.

This was referenced Mar 13, 2018
@jmvermeulen
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants