Agoda.IoC icon indicating copy to clipboard operation
Agoda.IoC copied to clipboard

Startup init for singletons

Open dicko2 opened this issue 3 years ago • 7 comments

Autofac has this


    /// <summary>
    /// When implemented by a component, an instance of the component will be resolved
    /// and started as soon as the container is built. Autofac will not call the Start()
    /// method when subsequent instances are resolved. If this behavior is required, use
    /// an <c>OnActivated()</c> event handler instead.
    /// </summary>
    /// <remarks>
    /// For equivalent "Stop" functionality, implement <see cref="IDisposable"/>. Autofac
    /// will always dispose a component before any of its dependencies (except in the presence
    /// of circular dependencies, in which case the components in the cycle are disposed in
    /// reverse-construction order.)
    /// </remarks>
    public interface IStartable
    {
        /// <summary>
        /// Perform once-off startup processing.
        /// </summary>
        void Start();
    }

anything that inherits from it will init on startup.

It's a cool idea some something we use in Supply extranets at Agoda. There this IHostedService now as well for background workers too

https://docs.microsoft.com/en-us/aspnet/core/fundamentals/host/hosted-services?view=aspnetcore-6.0&tabs=visual-studio

Idea would be to do something like this if we follow the autofac approach

[RegisterSingleton]
public class MyBackgroundWorker : IMyBackgroundWorker, IStartupable
{
// ..
} 

And then call a GetService after app init, this would also require use to add a new extension method though something like "UseAgodaIoC", or more specific "StartupAgodaIoCSingletons".

Alternatively we could simply leverage the IHostedService and register them with a new attribute

[RegisterSingletonStartup]
public class MyBackgroundWorker : IHostedService
{
// ..
} 

// in registartion
services.AddHostedService<MyBackgroundWorker>();
//..

The problem here though is that AddHostedService has no support for registering an interface.

In most of our use cases the startup method is prewarming cache in singletons that are later used for data fetchnig (i.e. Repositories, etc). So i think this wont work for us as we need interfaced use in teh regestration to properly mock for unit testing, so we need something simple like the autofac example imo.

Thoughts?

dicko2 avatar Mar 05 '22 05:03 dicko2

Adding some context from slack

A suggestion was made to use a background worker to prewarm anything that needs prewarming.

Essentially having a central "warmup class"

I'm not against people doing this, but one of the principles that we built agoda ioc on was decentralization of responsibilities, The 4k line UnityConfiguration class in the Agoda website from many years ago comes to mind.

When we look at the example in one of the larger supply extranets where we want to apply this, there's about more than 20 services/repos that need prewarm (a lot less than unity configuration nightmare for sure), then i think about the unit test we would need to write.

var underTest = new PrewarmService(MockGeoRepository,MockLanguageRepository, ...);// passing in >20 mocks
// we also have static content checks that this would fail because the ctor has too many parameters
MockGeoRepository.Prewarm().Received(1);
MockLanguageRepository.Prewarm().Received(1);
// copy past this type of line 20 times

Maybe I am missing something, but i feel this is less elegant than the autofac solution.

dicko2 avatar Mar 06 '22 02:03 dicko2

I find the [RegisterSingletonStartup] approach quite limiting (you can only use it with hosted services), especially compared to what you can achieve with Autofac.

It even feels less "safe" compared just using the service collection extensions directly, because AddHostedService<THostedService>() has a type constraint for the generic which ensures compile time errors if you don't use it with an IHostedService implementation - but you can drop in the attribute on any class and potentially blow up in runtime (we can probably write analyzers for that though...).

szaboopeeter avatar Mar 06 '22 05:03 szaboopeeter

So @szaboopeeter you are suggesting we do something like

[RegisterSingleton]
public class MyRepoThatNeedsWarmup : IMyRepoThatNeedsWarmup, IStartupable
{
// ..
} 

As the approach, so it's just a singleton in terms of attribute, and we infer the Startup feature by the fact it is inheriting from IStartupable?

dicko2 avatar Mar 06 '22 07:03 dicko2

``This is my idea

Let's say on consumer of Agoda.Ioc have these class

[RegisterSingleton]
public class MyRepoThatNeedsWarmup1 : IMyRepoThatNeedsWarmup1, IStartableAsync
{
    // IStartableAsync implementation
     public async Task StartAsync(){
         // Do warmup 
     }
} 


[RegisterSingleton]
public class MyRepoThatNeedsWarmup2 : IMyRepoThatNeedsWarmup2, IStartableAsync
{
    // IStartableAsync implementation
     public async Task StartAsync(){
         // Do warmup 
     }
} 

All of these class were register as IStartableAsync

In Agoda.Ioc we provide new IHostedService called NeedsWarmupBackgroundWorker

    /// <summary>
    /// This service was registered by Agoda.Ioc if comsumer has IStartableAsync implementation class
    /// services.AddHostedService<NeedsWarmupBackgroundWorker>();
    /// </summary>

    public class NeedsWarmupBackgroundWorker : IHostedService
    {
        private readonly IEnumerable<IStartableAsync> _startableAsyncs;
        // inject all singleto services
        public cBackgroundWorker(IEnumerable<IStartableAsync> startableAsyncs)
        {
            _startableAsyncs = startableAsyncs;
        }
        public async Task StartAsync(CancellationToken cancellationToken)
        {
            foreach (var startableAsync in _startableAsyncs)
            {
                await startableAsync.StartAsync();
            }
        }
        public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask;
    }

This way before we start we can make sure every NeedsWarmup class will be done before app start

kchinburarat avatar Mar 08 '22 03:03 kchinburarat

@kchinburarat when is

StartAsync

Called? It's after app.Run()? Or no?

dicko2 avatar Mar 08 '22 07:03 dicko2

I think it will start on app.run and should finish before kestrel do port binding

kchinburarat avatar Mar 08 '22 11:03 kchinburarat

This is the execution step image

kchinburarat avatar Mar 08 '22 14:03 kchinburarat