Task.WhenAll timing out after x number of Tasks

Question

I have the following code that gets called from a Controller.

public async Task Execute()
{
    var collections= await _repo.GetCollections(); // This gets 500+ items

    List<Object1> coolCollections= new List<Object1>();
    List<Object2> uncoolCollections= new List<Object2>();

    foreach (var collection in collections)
    {
        if(collection == "Something")
        {
            var specialObject = TurnObjectIntoSpecialObject(collection);
            uncoolCollections.Add(specialObject);
        }
        else
        {
            var anotherObject = TurnObjectIntoAnotherObject(object);
            coolCollections.Add(anotherObject);
        }
    }

        var list1Async = coolCollections.Select(async obj => await restService.PostObject1(obj)); //each call takes 200 -> 2000ms
        var list2Async = uncoolCollections.Select(async obj => await restService.PostObject2(obj));//each call takes 300 -> 3000ms

        var asyncTasks = list1Async.Concat<Task>(list2Async);
        await Task.WhenAll(asyncTasks); //As 500+ 'tasks'
}

Unfortunately, I'm getting a 504 error after around 300 or so requests. I can't change the API the RestService calls so I'm stuck trying to make the above code more performant.

Changing Task.WhenAll to a foreach loop does work, and does resolve the time out but it's very slow.

My question is how can I make sure the above code does not timeout after x number of requests?

You're either crashing or dealocking the remote service. Making more requests typically results in *worse* performance. You'll have to limit the number of concurrent calls instead of executing all of them at once. One way to do this is to use `Parallel.ForEachAsync` with a DOP that doesn't crash the remote site — Panagiotis Kanavos, Nov 04 '22 at 12:18
As a side note the name [`object`](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/reference-types#the-object-type) has a special meaning in C#, so using it as an argument name can create confusion. — Theodor Zoulias, Nov 04 '22 at 13:47
Hey @TheodorZoulias don't worry I'm aware object has a purpose in C# — Greg Jones, Nov 05 '22 at 12:40
Greg would you like to edit the question and change the `object` name to something else (like `obj` for example), so that your code has less compile errors, and the readers are not confused? — Theodor Zoulias, Nov 05 '22 at 12:42
I was about to upvote, but the `uncoolCollections` name turned me off. That's a cool name, too cool actually, so that it steals the focus from the question itself. I think that the original names were preferable. Only the `object` name was problematic. — Theodor Zoulias, Nov 05 '22 at 13:43

Panagiotis Kanavos · Accepted Answer · 2022-11-04T12:32:37.937

Making more concurrent calls to a remote site or database doesn't improve throughput, quite the opposite. Conflicts between the concurrent operations mean that beyond a certain point everything will start taking more time, until the service crashes. Right now you have a possible 300-way blocking problem.

The way all services handle this is by restricting the number of concurrent connections. In fact, many services will throttle clients so they don't crash if someone ... sends 500 concurrent requests. Some may even tell you they're throttling you with a 429 response.

Another way is to use batch requests, so instead of making 300 or 500 calls you can send a batch of 500 operations, allowing the service to handle them in an efficient way.

You can use Parallel.ForEachAsync to execute multiple calls with a specified degree of parallelism :

ParallelOptions parallelOptions = new()
{
    MaxDegreeOfParallelism = 30
};
 
await Parallel.ForEachAsync(object1, parallelOptions, async obj =>
{
    await restService.PostObject1(obj);
});
await Parallel.ForEachAsync(object2, parallelOptions, async obj =>
{
    await restService.PostObject2(obj);
});

You can adjust the DOP to find what works best without slowing down the remote service.

If the service can handle it, you could start both pipelines concurrently and await both of them to complete:

ParallelOptions parallelOptions = new()
{
    MaxDegreeOfParallelism = 30
};
 
var task1=Parallel.ForEachAsync(object1, parallelOptions, async obj =>
{
    await restService.PostObject1(obj);
});
var task2=Parallel.ForEachAsync(object2, parallelOptions, async obj =>
{
    await restService.PostObject2(obj);
});

await Task.WhenAll(task1,task2);

It doesn't make sense to retry those operations before you limit the DOP. Retrying 500 failed requests will only lead to another failure. Retrying with a random or staggered delay is essentially the same as limiting the DOP from the start, except it takes far longer to complete.

M.B. · Answer 2 · 2022-11-04T12:25:51.097

Since you are unable to change the rest service the 504 gateway timeout will remain. A better solution would be to use a retry mechanism, if you receive a 504 error code then you'll retry after 'x' seconds.

Why retry after 'x' seconds? The reasons for the 504 could be many, it could be that the server does not handle more request because it is at it's maximum workload at the moment.

A good and battle-tested library for retry mechanism is Polly.

You could also write your own function or action depending on the return type.

Depending on the happyflow use-case other methods could be used, but in this situation I went with the thought of you wanting to upload all the data even if an exception occurs.

Something to think about, if this service is provided by a third party vendor then look into the documentation. It will most likely have a section on max concurrent connections to the service.

score 0 · Answer 3 · answered Nov 04 '22 at 14:41

This answer assumes that you are using .NET 6 or later. You could project the objects to an enumerable of object elements, and then parallelize the processing of the projected objects with the Parallel.ForEachAsync method. This method allows to configure the MaxDegreeOfParallelism, so that not all projected objects are processed at once. As a result the remote server will not be bombarded with more requests than it can handle, nor the network bandwidth will be saturated. The optimal MaxDegreeOfParallelism can be found by experimentation. Start with a small number, like 5, and then gradually increase it until you find the sweet spot that offers the best performance.

public async Task Execute()
{
    var objects = await _repo.GetObjects();
    IEnumerable<object> projected = objects.Select(obj =>
    {
        if (obj == "Something")
        {
            return (object)TurnObjectIntoSpecialObject(obj);
        }
        else
        {
            return (object)TurnObjectIntoAnotherObject(obj);
        }
    });

    ParallelOptions options = new()
    {
        MaxDegreeOfParallelism = 5
    };
    await Parallel.ForEachAsync(projected, options, async (item, ct) =>
    {
        switch (item)
        {
            case Object1 obj1: await restService.PostObject1(obj1); break;
            case Object2 obj2: await restService.PostObject2(obj2); break;
            default: throw new NotImplementedException();
        }
    });
}

The above code processes the objects in the same order that appear in the objects sequence. The PostObject1/PostObject2 operations are parallelized, but the TurnObjectIntoSpecialObject/TurnObjectIntoAnotherObject operations are not. If you want to parallelize these too, then you can feed the Parallel.ForEachAsync with the objects, and do the projection inside the parallel loop.

In case of errors, only the first exception will be propagated. If you want to propagate all the errors, you can find solutions here.

Task.WhenAll timing out after x number of Tasks

3 Answers3