Mixing attributes

[yevhen]

There are a number of attribute combinations which doesn't make sense to mix together. For example: mixing StatelessWorker with any of Placement attributes doesn't make sense, but it looks like currently it is allowed to combine them within same class declaration.

I'm not sure what will happen in this case and I guess that handling code is smart enough to just ignore placement attributes if used along with StatelessWorker. Nevertheless, I found it to be confusing and I think it might be better to fail with exception to prevent users polluting grain declarations with useless combinations of attributes, which will confuse all future readers. Also, such validation should prevent users from cargo-culting Orleans.

Other invalid combinations:

• Mixing attributes from same category - looks like now I can put both RandomPlacement and ActivationCountBased or PreferLocal. Which one will Orleans choose?
• Reentrant could be mixed with AlwaysInterleave - seems like another confusing combination. Grain is either fully reentrant (class-level attribute) - all methods are subject to interleaving, or only some individual methods allow interleaving.

Another, possible and probably more intuitive approach will be, is to group all choices under respective category attributes (and use AllowMultiple=false), to make alternation explcit. Something like:

• Activation - Singleton / StatelessWorker
• Concurrency - Strict / Reentrant
• Placement - Random / PreferLocal / ActivationCountBased
• Delivery - Ordered / Unordered
• etc

What do you think?

[pherbel]

+1
This is more straightforward approach I like it

[gabikliot]

I like the straightforward approach. Maybe the naming for some categories will be a different.

[yevhen]

@gabikliot sure, it was just an example

[sergeybykov]

Should we catch such violations at compile/codegen time, just like we do for grain interface rules?

[yevhen]

@sergeybykov hmmm, but isn't codegen only operates on interface project? And those attributes target grain class implementations ...

[sergeybykov]

Codegen runs for grain implementations too, to generate state classes from state interfaces.

[sergeybykov]

To clarify, I meant to say that in addition to structuring attributes into categories and having compiler enforce AllowMultiple=false, which I think is a great suggestion, we can also catch any violations that pass that filter at codegen time.

[jkonecki]

Another +1 for grouping attributes.

On Tue, 10 Feb 2015 22:10 Sergey Bykov notifications@github.com wrote:

Codegen runs for grain implementations too, to generate state classes from
state interfaces.

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

[yevhen]

@sergeybykov ah, sorry, forgot about that. I don't use built-in persistence :) Ye, it would be great, since grouping into category attributes with backed choice enums is not enough to catch invalid combinations across categories, such as StatelessWorker + Placement, or Reentrant + AlwaysInterleave

[yevhen]

From my PoC, I've think that best way to group attributes is by having a configuration attribute for each type of activation. That will elegantly restrict what could be configured for each type of activation:

public enum Placement
{
    Auto,
    PreferLocal,
    DistributeEvenly
}

[AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = true)]
public class GrainAttribute : Attribute
{
    public readonly Placement Placement;

    public GrainAttribute(Placement placement = Placement.Auto)
    { 
        Placement = placement; 
    }
}

[AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = true)]
public class StatelessWorkerAttribute : Attribute
{
    // no additional configuration options at the moment
}

That only leaves us with mixing Reentrant and AlwaysInterleave but that not a big deal at all.