Using Parallel ForEach to POST in API and single Handle each Response

Question

I would like to know if the code I produced is good practice and does not produce leaks, I have more than 7000 objects Participant which I will push individually and Handle the Response to save the "external" id in our database. First I use the Parallel ForEach on the list pPartcipant:

Parallel.ForEach(pParticipant, participant =>
{
try
{
    //Create
    if (participant.id == null)
    {
        ExecuteRequestCreate(res, participant);
    }
    else
    {//Update
        ExecuteRequestUpdate(res, participant);
    }
}
catch (Exception ex)
{
    LogHelper.Log("Fail Parallel ", ex);
}
});

Then I do a classic (not async request), but after I need to "handle" the response (print in the console, save in a text file in async mode, and Update in my database)

    private async void ExecuteRequestCreate(Uri pRes, ParticipantDo pParticipant)
    {
        try
        {
            var request = SetRequest(pParticipant);

            //lTaskAll.Add(Task.Run(() => { ExecuteAll(request, pRes, pParticipant); }));
            //Task.Run(() => ExecuteAll(request, pRes, pParticipant));
            var result = RestExecute(request, pRes);
            await HandleResult(result, pParticipant);
            //lTaskHandle.Add(Task.Run(() => { HandleResult(result, pParticipant); }));
        }
        catch (Exception e)
        {
            lTaskLog.Add(LogHelper.Log(e.Message + " " + e.InnerException));
        }
    }

Should I run a new task for handeling the result (as commented) ? Will it improve the performance ? In comment you can see that I created a list of tasks so I can wait all at the end (tasklog is all my task to write in a textfile) :

       int nbtask = lTaskHandle.Count;
        try
        {
            Task.WhenAll(lTaskHandle).Wait();
            Task.WhenAll(lTaskLog).Wait();
        }

        catch (Exception ex)
        {
            LogHelper.Log("Fail on API calls tasks", ex);
        }

I don't have any interface it is a console program.

1) Parallel class was designed for CPU intensive jobs, not for I/O bound parallelism 2) Do not use async void prefer async Task 3) Do not use Wait() prefer await — Peter Csala
– Peter Csala, Commented Mar 11, 2021 at 14:43
@PeterCsala Thanks, but what's inside the loop does it matter that it is an async function ? — Benoît S
– Benoît S, Commented Mar 11, 2021 at 14:54
I used a Wait() method to purposely wait the end of all tasks and finish properly the program since I do some logs after each call I cannot close the program. I am aware of the difference between wait() and await :) — Benoît S
– Benoît S, Commented Mar 11, 2021 at 14:58
Parallel.Foreach does NOT have any overload which accepts async method. If you wish to perform I/O operations concurrently then please prefer await Task.WhenAll — Peter Csala
– Peter Csala, Commented Mar 11, 2021 at 14:59
First and foremost, avoid async void. Currently the Parallel class has no support for parallelizing asynchronous workloads. It's possible that a ForEachAsync method will be added in the next major .NET release. For now, you could take a look at multiple custom solutions to this problem. — Theodor Zoulias
– Theodor Zoulias, Commented Mar 11, 2021 at 14:59

Stephen Cleary · Accepted Answer · 2021-03-11 15:11:47Z

2

I would like to know if the code I produced is good practice

No; you should avoid async void and also avoid Parallel for async work.

Here's a similar top-level method that uses asynchronous concurrency instead of Parallel:

var tasks = pParticipant
    .Select(participant =>
    {
      try
      {
        //Create
        if (participant.id == null)
        {
          await ExecuteRequestCreateAsync(res, participant);
        }
        else
        {//Update
          await ExecuteRequestUpdateAsync(res, participant);
        }
      }
      catch (Exception ex)
      {
        LogHelper.Log("Fail Parallel ", ex);
      }
    })
    .ToList();
await Task.WhenAll(tasks);

And your work methods should be async Task instead of async void:

private async Task ExecuteRequestCreateAsync(Uri pRes, ParticipantDo pParticipant)
{
  try
  {
    var request = SetRequest(pParticipant);
    var result = await RestExecuteAsync(request, pRes);
    await HandleResult(result, pParticipant);
  }
  catch (Exception e)
  {
    LogHelper.Log(e.Message + " " + e.InnerException);
  }
}

answered Mar 11, 2021 at 15:11

Stephen Cleary

460k78 gold badges721 silver badges858 bronze badges

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

9 Comments

Benoît S Over a year ago

Thank you, my question is : Awaiting the restexecute and Handle result will not affect the time execution ? I have many participant, I don't need to wait for one result participant to go to the next ?

Stephen Cleary Over a year ago

No; await doesn't affect time execution. The WhenAll pattern runs them all concurrently.

Benoît S Over a year ago

Than you :) by the way I get an error the lambda expression should be asyn too : var tasks = pParticipant.Select(async participant =>.....

Benoît S Over a year ago

Why not using a list of task ? List<Task>();

Stephen Cleary Over a year ago

@Benoît: Yes. Because ExecuteRequestCreateAsync uses a normal await, and the Select delegate uses a normal await, then each task in that list will not complete until those subtasks are complete. You may find my async intro helpful.

|

Collectives™ on Stack Overflow

Using Parallel ForEach to POST in API and single Handle each Response

1 Answer 1

9 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

9 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related