Eliminate GrainState

[ReubenBond]

In days of old, Orleans' persistence model would generate IGrainState implementations for consumer. That functionality has since been removed and consumers instead store grain state in concrete classes which inherit from GrainState.

I propose that we eliminate GrainState for the following reasons:

• It uses reflection in AsDictionary and SetAll, which is not ideal.
• It is more code to support
• It adds boilerplate to user code (I rather my grains implement Grain<SomePoco>)
• It seems to only exist in order to add the ETag property to a stateful grain.
• @yevhen would do it

The ETag property which motivates GrainState's existence can be moved to the Grain<T> class instead. Storage Providers can be updated to operate on object instead of GrainState, giving them more flexibility over serialization and unifying them with the serialization model used by the rest of Orleans.

[galvesribeiro]

I agree with all points except related to the ETag.

I think ETag is a Storage Provider's Domain and the SP is the only interested on it (if in fact it will use it) to handle conditions like concurrency (not the case in Orleans thanks to the Turn model) and caching, which is a SP responsibility to deal with it, not the grain neither the GrainState.

Have a POCO class like MyGrain : Grain<MyPOCO> is something very cool.

[sergeybykov]

This is pretty much the same as #865. I agree that there benefits in getting rid of GrainState as the base class requirements. We could also consider if want to keep State as a property of have silo pass it to grain explicitly, e.g. via OnActivateAsync().

[gabikliot]

+1 for POCO state.

[galvesribeiro]

@sergeybykov keep the State property is not an issue... it will always be T, so I don't see a reason to remove it. Its a nice API IMHO...

[shayhatsor]

+1 POCO state
+1 keeping the state property
+1 etag is the SP domain (maybe we should have an optional IHaveEtag or ETagged< T >)
i also think that the SP methods should return an object, the next state. because currently the SP needs to somehow populate the given parameter state, which isn't ideal. the only problem with this implementation is that grain methods should be aware of that and not keep a reference to the state locally between calls to SP bridged methods. which seems to me like a small price to pay.
EDIT : also the current populate method can produce unexpected results in a reentrant grain. it's better to replace the current state in one step.

[ReubenBond]

Storage Providers are currently stateless, which is why they can't easily hold ETags or other per-grain context information.

What if providers supply their own IGrainState instance which Grain holds a reference to, any objections to that approach?

public interface IGrainState
// ^^^ Use an interface instead of a class. The class is for provider use.
{
    object State {get; set; }
}

public class ETaggedGrainState : IGrainState
{
    public object State { get; set; }

    public string ETag { get; set; }
}

public interface IStorageProvider : IProvider
{
    Logger Log { get; }

    /// <summary>
    /// Creates a new <see cref="IGrainState"/> instance for use by this provider.
    /// </summary>
    IGrainState CreateGrainState();
// ^^^ Add this method.


    Task ReadStateAsync(string grainType, GrainReference grainReference, IGrainState grainState);
    Task WriteStateAsync(string grainType, GrainReference grainReference, IGrainState grainState);
    Task ClearStateAsync(string grainType, GrainReference grainReference, IGrainState grainState);
}

[shayhatsor]

@ReubenBond I think that Storage Providers should remain stateless.

What do you think about this:

public interface IGrainState
{
    object State { get; set; }
    string ETag { get; set; }
}

public class GrainState<T> : IGrainState
{
    public T State { get; set; }

    object IGrainState.State
    {
        get { return State; }
        set { State = (T) value; }
    }

    public string ETag { get; set; }
}

public class Grain<T> : Grain
{
    protected Grain()
    {
        GrainState = new GrainState<T>();
    }

    protected Grain(IGrainIdentity identity, IGrainRuntime runtime, GrainState<T> state, IStorage storage)
        : base(identity, runtime)
    {
        GrainState = state;
        Storage = storage;
    }

    protected T State
    {
        get { return (T) GrainState.State; }
        set { GrainState.State = value; }
    }

    protected virtual Task ClearStateAsync()
    {
        return Storage.ClearStateAsync();
    }

    protected virtual Task WriteStateAsync()
    {
        return Storage.WriteStateAsync();
    }

    protected virtual Task ReadStateAsync()
    {
        return Storage.ReadStateAsync();
    }
}
public interface IStorageProvider : IProvider
{
    Logger Log { get; } 
    //Task ReadStateAsync(string grainType, GrainReference grainReference, GrainState grainState); 
    Task ReadStateAsync(Type grainType, GrainReference grainReference, IGrainState grainState);
    //Task WriteStateAsync(string grainType, GrainReference grainReference, GrainState grainState); 
    Task WriteStateAsync(Type grainType, GrainReference grainReference, IGrainState grainState); 
    //Task ClearStateAsync(string grainType, GrainReference grainReference, GrainState grainState);
    Task ClearStateAsync(Type grainType, GrainReference grainReference, IGrainState grainState);
}

EDIT : fixed a mistake i made with adding an interface with properties to a grain

[ReubenBond]

lgtm

On 24 November 2015 at 11:30, Shay Hazor notifications@github.com wrote:

@ReubenBond https://github.com/ReubenBond I think that Storage
Providers should remain stateless.
What do think about this:

public interface IGrainState
{
object State { get; set; }
string ETag { get; set; }
}
public class GrainState : IGrainState
{
public T State;

object IGrainState.State { get { return State; } set { State = (T) value; } }

public string ETag { get; set; }

}
internal interface IStatefulGrain
{
IGrainState GrainState { get; }
void SetStorage(IStorage storage);
}
public class Grain : Grain, IStatefulGrain
{
private IStorage storage; //removed from base class
private readonly GrainState grainState; //removed from base class

protected Grain()
{
    grainState = new GrainState<T>();
}

protected Grain(IGrainIdentity identity, IGrainRuntime runtime, GrainState<T> grainState, IStorage storage)
    : base(identity, runtime)
{
    this.grainState = grainState;
    this.storage = storage;
}

protected T State { get { return grainState.State; } set { grainState.State = value; } }

protected virtual Task ClearStateAsync() { return storage.ClearStateAsync(); }
protected virtual Task WriteStateAsync() { return storage.WriteStateAsync(); }
protected virtual Task ReadStateAsync() { return storage.ReadStateAsync(); }

void IStatefulGrain.SetStorage(IStorage storage) { this.storage = storage; }

IGrainState IStatefulGrain.GrainState { get { return grainState; } }

}
public interface IStorageProvider : IProvider
{
Logger Log { get; }
//Task ReadStateAsync(string grainType, GrainReference grainReference, GrainState grainState);
Task ReadStateAsync(Type grainType, GrainReference grainReference, IGrainState grainState);
//Task WriteStateAsync(string grainType, GrainReference grainReference, GrainState grainState);
Task WriteStateAsync(Type grainType, GrainReference grainReference, IGrainState grainState);
//Task ClearStateAsync(string grainType, GrainReference grainReference, GrainState grainState);
Task ClearStateAsync(Type grainType, GrainReference grainReference, IGrainState grainState);
}

—
Reply to this email directly or view it on GitHub
#1035 (comment).

[armona]

Few things I might add:
Add parameterless constructor constraint to the state objects, like so:

public class GrainState<T> : IGrainState, new()

This will preserve the current behavior that newly created grains can decide how to construct the 'initial state' on first activations (which in my opinion is useful).

Also, combining issue #895 (both will break the current IStorageProvider) - consider the following changes to the IStorageProvider interface:

public interface IStorageProvider : IProvider
{
    Logger Log { get; } 

    //Task ReadStateAsync(string grainType, GrainReference grainReference, GrainState grainState); 
    Task ReadStateAsync(Type grainType, GrainReference grainReference, Stream  serializedGrainState);

    //Task WriteStateAsync(string grainType, GrainReference grainReference, GrainState grainState); 
    Task WriteStateAsync(Type grainType, GrainReference grainReference, Stream  serializedGrainState); 

    //Task ClearStateAsync(string grainType, GrainReference grainReference, GrainState grainState);
    Task ClearStateAsync(Type grainType, GrainReference grainReference, Stream serializedGrainState);
}

This will allow decoupling serialization/compression from the persistence layer (double win).

And changing more attribues in the form:

[StorageProvider(ProviderName="AzureStore"), SerializationProvider(ProviderName="MySerializer"), CompressionProvider(ProviderName="Snappy")]

This will require to adjust these Grain methods accordingly:

    protected virtual Task ClearStateAsync()
    {
        return Storage.ClearStateAsync();
    }

    protected virtual Task WriteStateAsync()
    {
        return Storage.WriteStateAsync();
    }

    protected virtual Task ReadStateAsync()
    {
        return Storage.ReadStateAsync();
    }

[dVakulen]

You meant public class GrainState<T> : IGrainState where T: new()?, because currently there's no ability to initialize GrainState<T> field of the Grain<T> from user code inside grain.
As for combining with issue #895 and changing IStorageProvider interface - I don't think that every storage provider will be interested in serialized values e.g. the memory storage do not serialize objects in order to store them and not yet implemented EF provider could store state in relational table instead of the key-value one. Anyway, that's definitely subject for a separate PR.

[armona]

@dVakulen - Yes, I forgot the "where" keyword.

Yes I agree that not every storage provider would be interested in receiving serialized values. But currently there is no way to decouple the read/write operations without creating a common abstract storage provider, since every storage provider is expecting to get "un-serialized" objects.

[johnazariah]

By the way, guys - this is huge because, among other things, people can now do their domain models in F#, and use Orleans for the service/storage!

Very powerful!

Thank you!

[dVakulen]

@armona Using of the parameterless constructor constraint on grain state will prevent users from using F# DU types, and will break some already existing code.

[sergeybykov]

Resolved by #1060.