12

I have .NET core Web API which as service layer. Service layer has all EF code.

If have basecontroller with this code

protected Task<IActionResult> NewTask(Func<IActionResult> callback)
{
    return Task.Factory.StartNew(() =>
    {
        try
        {
            return callback();
        }
        catch (Exception ex)
        {
            Logger.LogError(ex.ToString());
            throw;
        }
    });
}

In controller action I wrap all calls to service in above method e.g. :

[HttpGet("something")]
public async Task<IActionResult> GetSomething(int somethingId)
{
    return await NewTask(() =>
    {
        var result = _somethingService.GetSomething(somethingId);

        if (result != null)
            return Ok(result);
        else
            return NotFound("Role not found");
    });
}

Is this correct pattern considering tomorrow I may have more than one service calls in action or making calls to other webservice. Please advise.

Vadim Ovchinnikov
  • 13,327
  • 5
  • 62
  • 90
krishnakumar
  • 617
  • 1
  • 6
  • 22
  • 4
    Can you please give more information about what you are trying to accomplish here? Your code basically now fakes asynchronous work by doing synchronous work on a background thread. Not to mention using Task.Factory.StartNew is pretty dangerous and you should be using Task.Run instead, but you don't need even need Task.Run here. – Kerim Emurla Feb 16 '17 at 14:08
  • something service has some crud operations which use entityframework core. also tommorrow i may have a service called feedService which will have calls to get feed from external web using httpclient. i want my api to benefit from async await thing.does above pattern will serve these needs + any concerns or improvements.? – krishnakumar Feb 16 '17 at 14:13

2 Answers2

26

i want my api to benefit from async await thing.does above pattern will serve these needs

No, it does not. Running synchronous work on the thread pool gives you the drawbacks of synchronous and asynchronous code, with the benefits of neither.

something service has some crud operations which use entityframework core

Currently, your action method is what I call "fake asynchronous" - it looks asynchronous (e.g., using await), but in fact is just running blocking code on a background thread. On ASP.NET, you want true asynchrony, whicn means you must be async all the way. For more about why this is bad on ASP.NET, see the first half of my intro to async on ASP.NET article (it mostly deals with ASP.NET non-core, but the first part talking about synchronous vs asynchronous requests is valid for any kind of server).

To make this truly asynchronous, you should start at the lowest level - in this case, your EFCore calls. They all support asynchrony. So, replace API calls like x.FirstOrDefault() with await x.FirstOrDefaultAsync() (and the same for all your creates/updates/deletes, etc).

Then allow async/await to grow naturally from there; the compiler will guide you. You'll end up with asynchronous methods on your somethingService which can be consumed as such:

[HttpGet("something")]
public async Task<IActionResult> GetSomething(int somethingId)
{
  var result = await _somethingService.GetSomethingAsync(somethingId);
  if (result != null)
    return Ok(result);
  else
    return NotFound("Role not found");
}
Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • ef core async calls are slower than sync calls please advise. – krishnakumar Feb 16 '17 at 21:43
  • @user1603828: The difference should be negligible. If that's not what you're seeing, I recommend contacting the EF team about it. – Stephen Cleary Feb 16 '17 at 22:04
  • 3
    EF 6 has negligible difference between async and sync calls, however the difference betwen async and sync calls in EF Core is pretty big. I'm sure they will fix that in the near future, but my advise would be to consider if you are really that concerned about performance and go with the async calls. [here](https://github.com/aspnet/EntityFramework/issues/5816) you can see some tests about how big is the difference between async and sync call in EF Core. – Kerim Emurla Feb 17 '17 at 08:41
  • @krishna I'm going to post the link to that podcast again so that anyone reading your comment here can see it. There is a difference between performance and throughput, and Kathleen describes how async should be at most a 10% overhead on performance but more pronounced performance on throughput. https://www.dotnetrocks.com/?show=1433 – thinklarge Jul 13 '17 at 23:05
6

Okay, first of all, you should stop using Task.Factory.StartNew and use Task.Run only when you have heavy CPU-bound work that you want to run on a thread pool thread. In you case you don't really need that at all. Also you should remember that you should only use Task.Run when calling a method and not in the implementation of the method. You can read more about that here.

What you really want in your case is to have asynchronous work inside your service (I'm not really sure you even need a service in your case) when you are actually making a call to the database and you want to use async/await and not just run some stuff on a background thread.

Basically your service should look something like this (if you are sure you need a service):

class PeopleService
{
    public async Task<Person> GetPersonByIdAsync(int id)
    {
        Person randomPerson = await DataContext.People.FirstOrDefaultAsync(x => x.Id == id);
        return randomPerson;
    }
}

As you can see your service now makes async calls to the database and that's basically what your pattern should be. You can apply this to all your operations(add/delete/ etc..)

After making your service asynchronous you should be easily able to consume the data in the action.

Your actions should look something like this:

[HttpGet("something")]
public async Task<IActionResult> GetPerson(int id)
{
    var result = await PeopleService.GetPersonByIdAsync(id);

    if (result != null)
        return Ok(result);
    else
        return NotFound("Role not found");
}
Vadim Ovchinnikov
  • 13,327
  • 5
  • 62
  • 90
Kerim Emurla
  • 1,141
  • 8
  • 15
  • i did bit research about ef core async methods and its said they are slower than sync ef methods.. please advise. – krishnakumar Feb 16 '17 at 21:42
  • @kirshna Async methods ARE slower. They have an overhead that are unavoidable when running them synchronously. BUT they can increase throughput. So there is a balance between both throughput and performance that you should be considering. As is always true of performance questions, it's complicated. https://www.dotnetrocks.com/?show=1433 Give that a listen and you'll see what I'm talking about. – thinklarge Jul 13 '17 at 23:02