Correct usage of Async/Await for Multiple Tasks To Db

Question

I have a simple scenario but I would like to know if my approach is correct, is it better advised to chose a single task to save my failed orders or can i kick off and fire off multiple tasks and wait for them all to complete. What is the correct approach for this scenario when it comes to connecting to a Db and saving entities.

I already have a single task based version of the below that saves one entity into the db.

    public async static Task SaveOrdersAsync(OrderService oService, OrderItemService oiService, IEnumerable<OrderTemplate> toSaveList, IUnitOfWork uow, IProgress<string> progress)
    {
        var toSave = toSaveList as IList<OrderTemplate> ?? toSaveList.ToList();
        var tasks = new Task[toSave.Count()];

        for (var i = 0; i < tasks.Length; i++)
        {
            var i1 = i;

            tasks[i] = new Task(() => SaveToDb(oService, oiService, toSave.ElementAt(i1), uow), TaskCreationOptions.PreferFairness);

            var message = string.Format("- Order: {0} has been resaved.\n", toSave.ElementAt(i1).Order.FriendlyId);

            if (progress != null)
                progress.Report(message);
        }

        await Task.WhenAll(tasks);
    }

At the moment, I have tested the above and believe that tasks have not started as the progress bar keeps looping around. My assumption is that Task.WhenAll should start my tasks for me - thats what I think?

or should be using this in the loop:

      tasks[i] = Task.Run(() => SaveToDb(oService, oiService, toSave.ElementAt(i1), uow));

I think I am close, just want someone to tell me if I a doing this correctly or not.

Feedback incorporated version:

    public async static Task SaveOrdersAsync(OrderService oService, OrderItemService oiService, IEnumerable<OrderTemplate> toSaveList, IUnitOfWork uow, IProgress<string> progress)
    {   
        var saveList = toSaveList as IList<OrderTemplate> ?? toSaveList.ToList();
        var saveTask = Task.Run(() =>
        {
            foreach (var ot in saveList)
            {
                SaveToDbBatch(oService, oiService, ot);

                var message = string.Format("- Order: {0} has been resaved.\n", ot.Order.FriendlyId);
                if (progress != null)
                    progress.Report(message);
            }
        });

        await saveTask;
        await Cache.UoW.SaveAsync();
    }

Stephen Cleary · Accepted Answer · 2014-11-22 15:13:11Z

6

What is the correct approach for this scenario when it comes to connecting to a Db and saving entities.

Generally speaking, you should:

Batch your saves, if possible. In other words, call a single method to update multiple records simultaneously. E.g., EF has SaveChangesAsync.
Use the natural async APIs for your database instead of Task.Run (or - even worse - the task constructor). E.g., EF has SaveChangesAsync.

answered Nov 22, 2014 at 15:13

Stephen Cleary

460k78 gold badges720 silver badges858 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

IbrarMumtaz Over a year ago

Cheers, stephen was hoping you were going to reply. Ok cool looks like i got some changes to make. =)

jt000 · Accepted Answer · 2014-11-22 15:09:14Z

Yes, you're correct that creating a task doesn't start it. Calling Task.Run(...) is the better option.

However, an even better option is to use the task that is returned from your call to ExecuteAsync(...) and await on that. This is because the ExecuteAsync task is an IO task & not a thread, so it executes differently and doesn't use up a thread pool thread.

As a side-note: Depending on the complexity of the "Save" it might be more reliable to do each "Save" consecutively. This is because if there are any database errors (like constraint violations) caused by a parallel task, then it will be extremely hard to reproduce if they are executed in parallel (i.e. at random times).

Excellent, I will consolidate this all down to one task, i didnt think of that.

usr · Accepted Answer · 2014-11-22 15:09:21Z

1

new Task(...) does not start the task. It is not the responsibility of Task.WhenAll to start them. The Task ctor should almost never be used.

Use Task.Run.

answered Nov 22, 2014 at 15:09

usr

172k38 gold badges251 silver badges380 bronze badges

Comments

Community · Accepted Answer · 2017-05-23 12:27:27Z

Seems like condolidating this down to one task as posted in my update worked and it also solved a side issue that I thought I would raise here incase some else is keen on pursuing my original approach. But i agree with @jaytre, that depending on the complexity of your saves and the object being saved it might be better to do each save consecutively for error handling - but thats up to you.

So if you pursue my original approach you might run into this error:

An EdmType cannot be mapped to CLR classes multiple times. The EdmType 'FrootPipe.Data.Order' is mapped more than once.

Which was basically down to a locking/synchronization issue - so different tasks were accessing the model at more or the less same time as one another, all trying to re-add a failed order back into the data model. So the error for my scenario is a little hard to tell but some googling lead me to the below.

For further reading see here: Entity framework MappingException: The type 'XXX has been mapped more than once

Collectives™ on Stack Overflow

Correct usage of Async/Await for Multiple Tasks To Db

4 Answers 4

1 Comment

1 Comment

Comments

Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

1 Comment

1 Comment

Comments

Comments

Linked

Related