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

How can I use an INotificationHandler as singleton instance? #519

Closed
lzjslqq opened this issue May 19, 2020 · 9 comments
Closed

How can I use an INotificationHandler as singleton instance? #519

lzjslqq opened this issue May 19, 2020 · 9 comments

Comments

@lzjslqq
Copy link

lzjslqq commented May 19, 2020

by default, MediatR will create an INotificationHandler instance when Publish method is called.
What I want is that MediatR call the INotificationHandler singleton instance once .
Anyone can show me the usage??
Thanks!!

@joaomatossilva
Copy link
Contributor

If I recall correctly that is all up to how you do register your handlers.
It's not MediatR that creates instances, but rather it asks your DI framework of choice for and instance that implements the handler interface you are requesting.

Just pointing that out in order to give you the pointers to where you should look. It's not the MediatR by itself, but rather the registering part on your application startup

@lilasquared
Copy link
Contributor

lilasquared commented May 19, 2020

If you are using the MediatR.Extensions.Microsoft.DependencyInjection library then it this library that is taking care of the registration for you. Depending on your use case I would suspect that you may not actually need the INotificationHandler to be the singleton, but a singleton dependency into that handler in order to accomplish your goal.

@lzjslqq
Copy link
Author

lzjslqq commented May 19, 2020

If you are using the MediatR.Extensions.Microsoft.DependencyInjection library then it this library that is taking care of the registration for you. Depending on your use case I would suspect that you may not actually need the INotificationHandler to be the singleton, but a singleton dependency into that handler in order to accomplish your goal.

Let me show my case , it‘s’ a console app and registered some BackgroundService like this:

       public static IHostBuilder CreateHostBuilder(string[] args) =>
         Host.CreateDefaultBuilder(args)
           .ConfigureServices((ctx, services) =>
           {
               services.AddMediatR(cfg => cfg.AsSingleton(), typeof(TestNotification));
               services.AddHostedService<TestService>();
           }); 

And the TestService is like this:

   public class TestService : BackgroundService, INotificationHandler<TestNotification>
   {
       private readonly IMediator _mediator;

       public TestService(IMediator mediator)
       {
           _mediator = mediator;
           Console.WriteLine($"constructed a TestService: {mediator.GetHashCode()}  sid: {this.GetHashCode()}");
       }

       public Task Handle(TestNotification notification, CancellationToken cancellationToken)
       {
           Console.WriteLine($"received msg:{notification.Message}");
           return Task.CompletedTask;
       }

       protected override Task ExecuteAsync(CancellationToken stoppingToken)
       {
            //  something to do
       }
   }

In thie case, everytime other service publish the TestNotification, the TestService's constructor is called, This significantly increases object allocation and GC pressure for heavy workloads, but TestService has been registered as singleton service by .Net Core DI container.
So all I need is that the TestService should be only constructed once.

@lzjslqq
Copy link
Author

lzjslqq commented May 19, 2020

If I recall correctly that is all up to how you do register your handlers.
It's not MediatR that creates instances, but rather it asks your DI framework of choice for and instance that implements the handler interface you are requesting.

Just pointing that out in order to give you the pointers to where you should look. It's not the MediatR by itself, but rather the registering part on your application startup

Thank you for your reply and this is my case:
#519 (comment)

@lilasquared
Copy link
Contributor

None of what you have shown particularly describes your use case, just the way you have currently set up your classes. I don't see any reason for your notification handler and background service to be the same class. If they need to share functionality or communicate they should have some sort of way to communicate through dependencies IMO.

@jbogard
Copy link
Owner

jbogard commented May 19, 2020

^^ your background service should NOT be a notification handler. Build the types to do something else, keeping in mind a background service is registered as a singleton.

@lzjslqq
Copy link
Author

lzjslqq commented May 20, 2020

^^ your background service should NOT be a notification handler. Build the types to do something else, keeping in mind a background service is registered as a singleton.

I build a new NotificationHandler to handle notification now 。Is there any way I can control the life of NotificationHandler?I find INotificationHandler<> is registered as Transient in the source code.

@jbogard
Copy link
Owner

jbogard commented May 25, 2020

You can change this in the companion MediatR.Extensions.Microsoft.DependencyInjection library - but be VERY careful in doing so. You really should not need to change the lifecycle of handlers - instead you control the lifecycle of dependencies injected into the handlers. Handlers should not have state - and therefore should be transient.

@jbogard jbogard closed this as completed May 25, 2020
@Farenheith
Copy link

Farenheith commented May 5, 2024

Just to contribute to this thread, I wanted to show how I achieved having a Singleton handler:

services.AddSingleton<
            IRequestHandler<BrokerPaymentRequestCommand, Result<List<BrokerPaymentResponse>>>,
            BrokerPaymentHandler
        >();
services.AddMediatR(c => {
            c.RegisterServicesFromAssemblyContaining(typeof(BrokerPaymentRequestCommand));
            c.Lifetime = ServiceLifetime.Singleton;
        });

Just by registering the handler manually before calling AddMediatR, he stays singleton.

I also want to talk about our approach to creating applications with all services singleton (except the controllers):
I really like the idea of keeping all the services you can singleton. The fact that a class is stateless, doesn't imply directly that you have to have a transient class, as you don't need to keep any result stored in it.

I know there are some concerns, for example, with the database connection scope. In our case, we're using Dapper in a aspnet.core application and created a Database Session class to manage the connection using AsyncLocal. It helps to make the connection to be acquired as late as possible, and to be returned to the pool as soon as possible, letting the connection be associated only with the current Task flow, ie, exclusively with the current request, making the scope of the services irrelevant, at least for that aspect.

I know in .net we tend to use Request Scope or Transient injection to deal with request isolation, but using only singletons brings some good benefits:

  • The general guideline is not to keep any state information in any service. You either pass it by parameter on your stack or use an AsyncLocal instance to keep request information. It's arguably simpler than scope planning;
  • You have less overhead for each request, as all the services are already created, of course;
  • You avoid service scope planning, which may be quite enjoyable to do and we already take the time it needs by granted, but it does require some time and intellectual effort to make it right. Also, it's quite common to have bugs introduced due to some wrong scope definitions;
  • Less memory and garbage collector use, as we don't have to constantly instantiate and dispose lots of classes;

The use of AsyncLocal doesn't come without cost, though. It also has some overhead and I don't know when it starts to become greater than DI instantiation overhead. I mean I don't know any performance comparison between the number of instantiating classes vs the number of AsyncLocal uses. But at least for our scenarios, it made the application simpler to maintain and to keep performant.

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