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

Minimal Workers #64874

Open
bradygaster opened this issue Feb 5, 2022 · 12 comments
Open

Minimal Workers #64874

bradygaster opened this issue Feb 5, 2022 · 12 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Extensions-Hosting
Milestone

Comments

@bradygaster
Copy link
Member

Background and Motivation

I had this idea over the weekend when building a minimal API, that would be augmented with a BackgroundService later on for some background processes. I thought "it would be great to have something like a Minimal Workers sort of API. I mentioned it to @halter73 in an email to get his 2C, but wanted to put this out here for discussion.

Proposed API

This is in no way proposed to be final, but, hopefully the idea will come across. Let's say I have 3 different processes I want to run on intervals in the background of an ASP.NET app, but none of them are really big enough to warrant a whole new class. It'd be great if I could just do something like this in my Program.cs to wire up some background processes.

Usage Examples

builder.Services.AddBackgroundProcess(async (token) =>
{
    Console.WriteLine($"First Process Running at {DateTime.Now}");
    await Task.Delay(1000);
});

builder.Services.AddBackgroundProcess(async (token) =>
{
    Console.WriteLine($"Second Process Running at {DateTime.Now}");
    await Task.Delay(5000);
});

builder.Services.AddBackgroundProcess(async (token) =>
{
    Console.WriteLine($"Third Process Running at {DateTime.Now}");
    await Task.Delay(10000);
});

var app = builder.Build();

Very Brief Implementation and Usage

I wired up a super-simple implementation to support this usage scenario. This is in no way intended to be an actual proposal - surely it'd need some improvement and more consideration.

namespace Microsoft.Extensions.Hosting
{
    public class BackgroundProcessWrapper : BackgroundService
    {
        public BackgroundProcessWrapper(Func<CancellationToken, Task>? unitOfWork)
        {
            UnitOfWork = unitOfWork;
        }

        public Func<CancellationToken, Task>? UnitOfWork { get; set; }

        protected override async Task ExecuteAsync(CancellationToken stoppingToken)
        {
            if (UnitOfWork == null) return;

            while (!stoppingToken.IsCancellationRequested)
            {
                await UnitOfWork(stoppingToken);
            }
        }
    }

    public static class BackgroundServiceExtensions
    {
        public static IServiceCollection AddBackgroundProcess(this IServiceCollection services, Func<CancellationToken, Task>? process)
        {
            services.AddSingleton<IHostedService>((services) => new BackgroundProcessWrapper(process));
            return services;
        }
    }
}

Alternative Designs

I'm sure there's a better way to do this - just starting the conversation on a way to achieve "minimal workers" if other folks think there's value.

Risks

We'd need to support the ability to inject services from the IServicesCollection into the AddBackgroundProcess process Func, so folks could use their services within the ExecuteAsync phase.

Feedback welcome.

@bradygaster bradygaster added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Feb 5, 2022
@davidfowl davidfowl transferred this issue from dotnet/aspnetcore Feb 6, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Feb 6, 2022
@davidfowl
Copy link
Member

BackgroundProcessWrapper doesn't need to be public. I would probably also rename AddBackgroundProcess to something else, AddBackgroundService? Since it's just a BackgroundService.

@Kahbazi
Copy link
Member

Kahbazi commented Feb 7, 2022

Should there be two type of background service method? One for recurring which would be in a while loop and the other just run for once and let user decide how the service is run, it could be useful when listening to an events instead of timer.

Also the methods could get a Delegate like minimal API and provide CancellationToken and other services from a IServiceProvider. But the IServiceProvider in AddBackgroundService could be the main one but in AddRecurringBackgroundService a scope could be created and the services are resolved from its IServiceProvider.

public static class BackgroundServiceExtensions
{
    public static IServiceCollection AddBackgroundService(this IServiceCollection services, Delegate process);
    public static IServiceCollection AddRecurringBackgroundService(this IServiceCollection services, Delegate process);
}

And since this API in on IServiceCollection, it might be a little confusing for new users that why builder.Services.AddBackgroundService works but app.Services.AddBackgroundService don't.

@ghost
Copy link

ghost commented Feb 7, 2022

Tagging subscribers to this area: @dotnet/area-extensions-hosting
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and Motivation

I had this idea over the weekend when building a minimal API, that would be augmented with a BackgroundService later on for some background processes. I thought "it would be great to have something like a Minimal Workers sort of API. I mentioned it to @halter73 in an email to get his 2C, but wanted to put this out here for discussion.

Proposed API

This is in no way proposed to be final, but, hopefully the idea will come across. Let's say I have 3 different processes I want to run on intervals in the background of an ASP.NET app, but none of them are really big enough to warrant a whole new class. It'd be great if I could just do something like this in my Program.cs to wire up some background processes.

Usage Examples

builder.Services.AddBackgroundProcess(async (token) =>
{
    Console.WriteLine($"First Process Running at {DateTime.Now}");
    await Task.Delay(1000);
});

builder.Services.AddBackgroundProcess(async (token) =>
{
    Console.WriteLine($"Second Process Running at {DateTime.Now}");
    await Task.Delay(5000);
});

builder.Services.AddBackgroundProcess(async (token) =>
{
    Console.WriteLine($"Third Process Running at {DateTime.Now}");
    await Task.Delay(10000);
});

var app = builder.Build();

Very Brief Implementation and Usage

I wired up a super-simple implementation to support this usage scenario. This is in no way intended to be an actual proposal - surely it'd need some improvement and more consideration.

namespace Microsoft.Extensions.Hosting
{
    public class BackgroundProcessWrapper : BackgroundService
    {
        public BackgroundProcessWrapper(Func<CancellationToken, Task>? unitOfWork)
        {
            UnitOfWork = unitOfWork;
        }

        public Func<CancellationToken, Task>? UnitOfWork { get; set; }

        protected override async Task ExecuteAsync(CancellationToken stoppingToken)
        {
            if (UnitOfWork == null) return;

            while (!stoppingToken.IsCancellationRequested)
            {
                await UnitOfWork(stoppingToken);
            }
        }
    }

    public static class BackgroundServiceExtensions
    {
        public static IServiceCollection AddBackgroundProcess(this IServiceCollection services, Func<CancellationToken, Task>? process)
        {
            services.AddSingleton<IHostedService>((services) => new BackgroundProcessWrapper(process));
            return services;
        }
    }
}

Alternative Designs

I'm sure there's a better way to do this - just starting the conversation on a way to achieve "minimal workers" if other folks think there's value.

Risks

We'd need to support the ability to inject services from the IServicesCollection into the AddBackgroundProcess process Func, so folks could use their services within the ExecuteAsync phase.

Feedback welcome.

Author: bradygaster
Assignees: -
Labels:

api-suggestion, untriaged, area-Extensions-Hosting

Milestone: -

@davidfowl
Copy link
Member

One for recurring which would be in a while loop and the other just run for once and let user decide how the service is run, it could be useful when listening to an events instead of timer.

This should be a separate feature request. There's all sorts of questions here about scheduling.

And since this API in on IServiceCollection, it might be a little confusing for new users that why builder.Services.AddBackgroundService works but app.Services.AddBackgroundService don't.

Agree but I don't know what the fix is for that. Also, I'm not sure why the method takes a delegate? Is that to support DI parameters? What about scoped services? What about the cancellation token etc. etc.

@halter73
Copy link
Member

halter73 commented Feb 7, 2022

Here's the proposal for the new builder in runtime: #61634. This is also how it would look with WebApplicationBuilder.

I'm on the fence about the Delegate parameter. It doesn't seem as useful as it is with minimal APIs since the parameter binding wouldn't be as advanced, but I do see how it's a bit cleaner than taking an IServiceProvider. The cost is not being able to quickly determine what parameters are allowed by looking at the signature.

@maryamariyan maryamariyan removed the untriaged New issue has not been triaged by the area owner label Feb 8, 2022
@maryamariyan maryamariyan added this to the Future milestone Feb 8, 2022
@Kahbazi
Copy link
Member

Kahbazi commented Feb 9, 2022

One for recurring which would be in a while loop and the other just run for once and let user decide how the service is run, it could be useful when listening to an events instead of timer.

This should be a separate feature request. There's all sorts of questions here about scheduling.

I think I used the wrong term here, by recurring I meant the very same implementation that is in the proposed issue which run the delegate in a while loop, and the simple one just don't have the while loop.

I'm not sure why the method takes a delegate? Is that to support DI parameters? What about scoped services? What about the cancellation token etc. etc.

Yes, the delegate is to support the parameters injection from DI and for CancellationToken the stoppingToken from BackgroundService.ExecuteAsync could be send. Also each call to the delegate would be in its own scope.

internal class RecurringBackgroundProcessWrapper : BackgroundService
{
    private readonly IServiceScopeFactory _serviceScopeFactory;
    private readonly Func<IServiceProvider, CancellationToken, Task> _process;

    public RecurringBackgroundProcessWrapper(IServiceProvider serviceProvider, Delegate process)
    {
        _process = BackgroundServiceBuilder.Build(process);
        _serviceScopeFactory = serviceProvider.GetRequiredService<IServiceScopeFactory>();
    }

    protected override async Task ExecuteAsync(CancellationToken stoppingToken)
    {
        while (!stoppingToken.IsCancellationRequested)
        {
            var scope = _serviceScopeFactory.CreateScope();
            try
            {
                await _process.Invoke(scope.ServiceProvider, stoppingToken);
            }
            finally
            {
                if (scope is IAsyncDisposable asyncDisposable)
                {
                    await asyncDisposable.DisposeAsync();
                }
                else
                {
                    scope.Dispose();
                }
            }
        }
    }
}

internal class BackgroundProcessWrapper : BackgroundService
{
    private readonly IServiceProvider _serviceProvider;
    private readonly Func<IServiceProvider, CancellationToken, Task> _process;

    public BackgroundProcessWrapper(IServiceProvider serviceProvider, Delegate process)
    {
        _process = BackgroundServiceBuilder.Build(process);
        _serviceProvider = serviceProvider;
    }

    protected override async Task ExecuteAsync(CancellationToken stoppingToken)
    {
        // I'm not sure if this one is also needs a scope.
        // It seems the Microsoft implementation of IServiceProvider can inject scope service
        // when there's no scope but I'm not sure about the third party implementations.
        await _process.Invoke(_serviceProvider, stoppingToken);
    }
}

public static class BackgroundServiceExtensions
{
    public static IServiceCollection AddBackgroundProcess(this IServiceCollection services, Delegate process)
    {
        services.AddSingleton<IHostedService>((services) => new BackgroundProcessWrapper(services, process));
        return services;
    }

    public static IServiceCollection AddRecurringBackgroundProcess(this IServiceCollection services, Delegate process)
    {
        services.AddSingleton<IHostedService>((services) => new RecurringBackgroundProcessWrapper(services, process));
        return services;
    }
}

@davidfowl
Copy link
Member

So something that runs in a tight loop? Is that useful without being able to control an interval? Or is the idea that you also have to wait in your callback?

@Kahbazi
Copy link
Member

Kahbazi commented Feb 12, 2022

Or is the idea that you also have to wait in your callback?

Yes, this way the callback could even decide to have different interval if its needed.

Another idea is that the delegate could return a Timespan? which decide the next interval and if it is null the loop would stoped.

@halter73
Copy link
Member

halter73 commented Nov 11, 2022

I think we should overload AddHostedService. We also need a way to resolve services. Straw man API proposal:

Proposed API

// Microsoft.Extensions.Hosting.Abstractions.dll

namespace Microsoft.Extensions.DependencyInjection;

public static class ServiceCollectionHostedServiceExtensions
{
+    public static IServiceCollection AddHostedService(this IServiceCollection services, Func<IServiceProvider, CancellationToken> backgroundWorker)
}

Usage Examples

var builder = Host.CreateApplicationBuilder(args);

builder.Services.AddSingleton<MyService>();
builder.Services.AddHostedService(async (serviceProvider, stoppingToken) =>
{
    var myService = serviceProvider.GetRequiredService<MyService>();
    using var timer = new PeriodicTimer(TimeSpan.FromSeconds(10));

    while (await timer.WaitForNextTickAsync(stoppingToken))
    {
        service.Ping()
    }
});

builder.Build().Run();

Alternative Designs

We could pass in the IHost instead of the IServiceProvider, but I think 99% of IHost usage would be host.Services. And you can resolve the IHost from the service provider.

+    public static IServiceCollection AddHostedService(this IServiceCollection services, Func<IHost, CancellationToken> backgroundWorker)

Or we could pass in a Delagate as @Kahbazi suggested above and @julian-code in #78137.

+    public static IServiceCollection AddHostedService(this IServiceCollection services, Delegate backgroundWorker)

Risks

@Kahbazi points out an interesting risk above.

And since this API in on IServiceCollection, it might be a little confusing for new users that why builder.Services.AddBackgroundService works but app.Services.AddBackgroundService don't.

We could have the host register a parent hosted service by default that runs child hosted services added by an extension method on IServiceProvider, but adding an IServiceProvider extension method that does anything other than resolve a service seems like going way too far.

IHost might be a better target for an AddHostedService (or AddBackgroundService since we're no longer overloading) extension method that works this way. app.AddBackgroundService(...) feels reasonable.

@davidfowl
Copy link
Member

IHost might be a better target for an AddHostedService (or AddBackgroundService since we're no longer overloading) extension method that works this way. app.AddBackgroundService(...) feels reasonable.

Did you mean IHostBuilder?

@davidfowl
Copy link
Member

Or we could pass in a Delagate as @Kahbazi suggested above and @julian-code in #78137.

I think we should avoid this. There's too much infrastructure missing to do this properly and it will turn into a mini framework before we know it. We should do the basic overload and hold off on the Delegate overload.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Extensions-Hosting
Projects
None yet
Development

No branches or pull requests

6 participants