dotnet / orleans

Cloud Native application framework for .NET
https://docs.microsoft.com/dotnet/orleans
MIT License
10.05k stars 2.02k forks source link

Default provider pattern #4556

Open jason-bragg opened 6 years ago

jason-bragg commented 6 years ago

In the legacy provider model, providers could be acquired by name or a system could request the default. The default was the provider named “Default”. When we moved to using dependency injection, where keyed services were used instead of provider managers, systems could resolve the provider by name as a keyed service, or by the service type to get the default. For example:

        IGrainStorage storageProvider = string.IsNullOrEmpty(providerName)
            ? services.GetServiceByName<IGrainStorage>(providerName)  // get by name or
            : services.GetService<IGrainStorage>(); // get default

To support the legacy naming conventions, we continued using the pattern wherein naming a provider “Default” would signal that it’s the default provider. This is supported by adding a mapping of the provider interface to the ‘default’ named service into the container. Something like:

        services.TryAddSingleton<IGrainStorage>(sp => sp.GetServiceByName<IGrainStorage>(ProviderConstants.DEFAULT_STORAGE_PROVIDER_NAME));

This works but has some peculiarities.

Questions: Do we want to continue to support default providers? If so how do we want them to work? Does the above pattern fit our needs? Is there a better pattern?

jason-bragg commented 6 years ago

We currently add the mapping:

services.TryAddSingleton<IGrainStorage>(sp => sp.GetServiceByName<IGrainStorage>(ProviderConstants.DEFAULT_STORAGE_PROVIDER_NAME));

any time we add a grain storage, not just when the default is added. This is wasteful, but also seems to break some containers, as returning null for a registered service is considered invalid for some containers. I'm not sure what the 'conforming container' rules are around this, but it's sloppy in any case.

buddy-james commented 6 years ago

I'm trying to use Autofac and I'm running into an issue. If I use .AddAdoNetGrainStorageAsDefault then a deafult is set and I'm able to start the silo, however, I'm not able to subscribe to a simple message stream because it can't find a storage provider named "PubSubStore". If I configure the silo with .AddAdoNetGrainStorage("PubSubStore", ConfigureStorage), specifying "PubSubStore" as the name, then I receive the following exception.

fail: Orleans.Runtime.SiloLifecycleSubject[100450] Lifecycle observer Orleans.Runtime.Versions.GrainVersionStore failed to start due to errors at stage 10000. Autofac.Core.DependencyResolutionException: An error occurred during the activation of a particular registration. See the inner exception for details. Registration: Activator = IGrainStorage (DelegateActivator), Services = [Orleans.Storage.IGrainStorage], Lifetime = Autofac.Core.Lifetime.RootScopeLifetime, Sharing = Shared, Ownership = OwnedByLifetimeScope ---> A delegate registered to create instances of 'Orleans.Storage.IGrainStorage' returned null. (See inner exception for details.) ---> Autofac.Core.DependencyResolutionException: A delegate registered to create instances of 'Orleans.Storage.IGrainStorage' returned null. at Autofac.Core.Activators.Delegate.DelegateActivator.ActivateInstance(IComponentContext context, IEnumerable1 parameters) at Autofac.Core.Resolving.InstanceLookup.Activate(IEnumerable1 parameters) --- End of inner exception stack trace --- at Autofac.Core.Resolving.InstanceLookup.Activate(IEnumerable1 parameters) at Autofac.Core.Resolving.InstanceLookup.<Execute>b__5_0() at Autofac.Core.Lifetime.LifetimeScope.GetOrCreateAndShare(Guid id, Func1 creator) at Autofac.Core.Resolving.InstanceLookup.Execute() at Autofac.Core.Resolving.ResolveOperation.GetOrCreateInstance(ISharingLifetimeScope currentOperationScope, IComponentRegistration registration, IEnumerable1 parameters) at Autofac.Core.Resolving.ResolveOperation.Execute(IComponentRegistration registration, IEnumerable1 parameters) at Autofac.Core.Lifetime.LifetimeScope.ResolveComponent(IComponentRegistration registration, IEnumerable1 parameters) at Autofac.ResolutionExtensions.TryResolveService(IComponentContext context, Service service, IEnumerable1 parameters, Object& instance) at Autofac.ResolutionExtensions.ResolveOptionalService(IComponentContext context, Service service, IEnumerable`1 parameters) at Autofac.Extensions.DependencyInjection.AutofacServiceProvider.GetService(Type serviceType) at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetService[T](IServiceProvider provider) at Orleans.Runtime.Versions.GrainVersionStore.OnStart(CancellationToken token) in D:\build\agent_work\18\s\src\Orleans.Runtime\Versions\GrainVersionStore.cs:line 90 at Orleans.LifecycleExtensions.Observer.OnStart(CancellationToken ct) at Orleans.Runtime.SiloLifecycleSubject.MonitoredObserver.d__9.MoveNext() in D:\build\agent_work\18\s\src\Orleans.Runtime\Lifecycle\SiloLifecycleSubject.cs:line 70

@jason-bragg I believe this what you were referring to when you said

but also seems to break some containers, as returning null for a registered service is considered invalid for some containers.

Do you have any suggestions on how I can get past this issue? Do I need to stop using autofac?

Thank you!

buddy-james commented 6 years ago

It looks like I was able to resolve the issue by calling both: siloHostBuilder.AddAdoNetGrainStorage("PubSubStore", ConfigureStorage) .AddAdoNetGrainStorageAsDefault(ConfigureStorage)

jason-bragg commented 6 years ago

@buddy-james, Thanks for providing more context. and yup, that's the problem I was referring too. Your workaround is appropriate given the current patterns. I can't commit at this time to when we'll address this, but the issue is captured, so it's on our radar.

Eldar1205 commented 6 years ago

@jason-bragg Hi Jason, in the past I've participated in DI discussions and raised concerns regarding the conforming container anti-pattern, and what happened above with Autofac also happened with SimpleInjector which is the specific one I use; since the scene in DI container world evolves over time, I still think Orleans should assume DI framework friendly patterns as pointed out by Mark Seeman, and in particular Orleans can and probably should come with a DI container meant for Orleans as a framework, not to be mingled with the application's DI container if the application even uses one. The addition of IGrainActivator was a great step forward IMO, since now I can register to Orleans DI container a very simple service that resolves the grains which have complex object graphs from my application's DI container, and I like the fact that such a separation can allows Orleans DI to evolve as Orleans needs, and application's DI can evolve as the application needs

jason-bragg commented 6 years ago

@Eldar1205, Hi Eldar, Good to hear from you again. I understand your concerns, and agree that the DI container world is evolving, but disagree with Mark's framing of conforming containers as an anti-pattern.

In my opinion, simply Identifying shortcomings and naming something an anti-pattern does not make it so. If containers are to survive in the long term they'll need to conform to a minimal set of standard behaviors that frameworks can rely upon. If not, the technology will never achieve it's potential. Expecting framework and application developers to navigate an obstacle course of container orchestration rules, because containers can’t agree upon and conform to a minimum set of common behaviors sufficient to support frameworks and other shared tech, is impractical at best. The containers that survive will be those that agree upon an conform to a minimal common set of behaviors.

In the case of Orleans the issue is more complicated than Mark lays out. We don’t simply have the framework layer, and the application layer. We have the core framework, the framework extensions (like storage providers, stream providers, and other framework extensibility points) and application developers. We don’t have a well defined set of interfaces dividing these layers as code generated by the framework becomes part of the logic for framework, extensions, and application components. The container which is used to pull all of these parts together at runtime generates fairly hard to predict call patterns between the various layers, making the use of different containers much more challenging than most engineers would choose to face. Additionally configuring multiple containers to play nicely in this world would be error prone and greatly complicate the story.

Despite the above, we can do better with our interfaces, extensibility points, and how we handle custom containers. I’m not arguing that we’re fine.. we should do better. I’m only arguing that I disagree with the premise behind Mark’s position. Containers will need to conform to a minimal set of common behaviors. It’s impractical not to..

sergeybykov commented 6 years ago

@jason-bragg Let's open a separate issue for the null in DI, so that we don't mix the two issues here.

jason-bragg commented 6 years ago

4710