Fire and forget option when saving to storage in MiniProfilerMiddleware

[prat0088]

This line of code in MiniProfilerMiddleware eventually leads to this call to save the timing information to the storage provider. On each request.

It would be nice to have an option on the Middleware to invoke the StopAsync() as _ = mp.StopAsync().ConfigureAwait(false); or Task.Run(() => ...); so we don't block on the write to the provider, slowing down the request. Especially when the provider is a networked database.

An alternative would be to add that option to the provider, but I count 6 different networked database providers. The option would have to be added to each new provider created.

Would MiniProfiler's maintainers consider such a pull request as described in the second paragraph?

[NickCraver]

The issue really affects not just middleware but all usages, so IMO, it's just not the correct place to handle it.

However, given the interface for IAsyncStorage doesn't return anything on the saves, we could approach this in a more global way. For example by creating something like:

public class FireAndForgetStorage : IAsyncStorage
{
    private IAsyncStorage BackingStore { get; }
    public FireAndForgetStorage(IAsyncStorage backingStore) => BackingStore = backingStore;
    // ... proxy methods
}

By having such a generic wrapper, you could do something like:

options.Storage = new FireAndForgetStorage(
    new SqlServerStorage("Data Source=.;Initial Catalog=MiniProfiler;Integrated Security=True;")
);

Or, it could be an extension method, such as:

options.Storage = new SqlServerStorage("Data Source=.;Initial Catalog=MiniProfiler;Integrated Security=True;").FireAndForget();

(though maybe that's overkill)

Anyway, doing it at the storage level I feel is much more appropriate and flexible, but only some things are fire and forget (basically: things that return void). Thoughts on such an approach? This could be built into the main library and added to the docs - the individual providers wouldn't need to do anything.

[NickCraver]

@prat0088 any thoughts on the above approach?

[prat0088]

@NickCraver That would work perfect! Great solution. I prefer the first approach of FireAndForgetStorage() without extension method, but don't have a strong opinion.

[liqdfire]

I have a very basic version of that using a blocking collection and a looping task. This is a "best effort" i.e. it is not handling retries is there is an error with the save method. This is not full production ready either, it was just my first draft last night getting profiling minimally functional.

GetConsumingEnumerable will block if the collection is empty, and as soon as an item is added will return it to the for loop for processing. However, it could be possible depending on the number of requests per seconds to have the foreach running in a tight loop and consume a lot of CPU, some type of throttling mechanism may not be a bad idea.

public class BackgroundSqlServerStorage: IAsyncStorage
  {
    private readonly IAsyncStorage _storageProvider;
    private readonly BlockingCollection<MiniProfiler> _queue = new BlockingCollection<MiniProfiler>();

    private Task _loopTask;

    public BackgroundSqlServerStorage(IAsyncStorage storage)
    {
      _storageProvider = storage;
      _loopTask = Task.Run(WorkerLoop);
    }

    private async Task WorkerLoop()
    {
      try
      {
        foreach (var profiler in _queue.GetConsumingEnumerable())
        {
          await _storageProvider.SaveAsync(profiler);
        }
      }
      catch (Exception e)
      {
        //todo: log to logging service
        Console.WriteLine(e);
      }
    }

    public List<Guid> GetUnviewedIds(string user)
    {
      return _storageProvider.GetUnviewedIds(user);
    }

    public Task<List<Guid>> GetUnviewedIdsAsync(string user)
    {
      return _storageProvider.GetUnviewedIdsAsync(user);
    }

    public IEnumerable<Guid> List(int maxResults, DateTime? start = null, DateTime? finish = null, ListResultsOrder orderBy = ListResultsOrder.Descending)
    {
      return _storageProvider.List(maxResults, start, finish, orderBy);
    }

    public Task<IEnumerable<Guid>> ListAsync(int maxResults, DateTime? start = null, DateTime? finish = null, ListResultsOrder orderBy = ListResultsOrder.Descending)
    {
      return _storageProvider.ListAsync(maxResults, start, finish, orderBy);
    }

    public MiniProfiler Load(Guid id)
    {
      return _storageProvider.Load(id);
    }

    public Task<MiniProfiler> LoadAsync(Guid id)
    {
      return _storageProvider.LoadAsync(id);
    }

    public void Save(MiniProfiler profiler)
    {
      _queue.Add(profiler);
    }

    public Task SaveAsync(MiniProfiler profiler)
    {
      _queue.Add(profiler);
      return Task.CompletedTask;
    }

    public void SetUnviewed(string user, Guid id)
    {
      _storageProvider.SetUnviewed(user, id);
    }

    public Task SetUnviewedAsync(string user, Guid id)
    {
      return _storageProvider.SetUnviewedAsync(user, id);
    }

    public void SetViewed(string user, Guid id)
    {
      _storageProvider.SetViewed(user, id);
    }

    public Task SetViewedAsync(string user, Guid id)
    {
      return _storageProvider.SetViewedAsync(user, id);
    }
  }