Downloading blobs asynchronously

Question

We have a working implementation that is doing I/O operations, returning data from blobs in an async/await manner from Azure Blob Storage.

//Method1 is not async, this is what is called from the controller
public IEnumerable<Data> Method1()
{
    //Running the async method and returning the result from task
    return Task.Run(() => GetDataAsync()).Result;
}


private async Task<IEnumerable<Data>> GetDataAsync()
{
    //There are multiple blob address where the data is held. The code creates, in parallel multiple tasks for each address.
    //It returns tasks that will be run in Async pattern
    var tasks = multipleBlobAddress.AsParallel.Select(blobAddress =>{
            Task<IEnumerable<Data>> task = GetDataFromBlobsAsync(blobAddress);
            return task;
    });

    //Awaits all tasks to complete
    var completedTasks = await Task.WhenAll<IEnumerable<Data>>(tasks);
    //Selects all the tasks and returns them. Each tasks has data.
    return completedTasks.SelectMany(t => t);
}

private Task<IEnumerable<Data>> GetDataFromBlobsAsync(string address)
{
    //Opens the blob and reads from it
    using (var blobStream = blobService.OpenRead(address))
    {
        //Deserialises the data read from the blob
        data = service.DeserialiseData(blobStream);
    }

    return Task.FromResult<IEnumerable<Data>>(data);
}

We have understood that the best way to read from blobs is to follow the async/await pattern and not using the AsParallel method (other suggestions are most welcomed). I have the following questions:

1. By creating tasks in parallel and then waiting for all of them to complete, do we lose any performance?
2. What are the obvious areas where we got it wrong or we can improve and potentially increase the performance or make the code better to maintain/read?
3. Are we correctly following the async / await pattern?

If you need any extra information about the code, I will happily provide it and hope to make it clearer.

Answer 1

There is never a need to do this:

return Task.Run(() => GetDataAsync()).Result;

Why start a new task and then synchronously wait with .Result? This is the sort of code that will bite you later when you find out that a synchronisation context can cause a deadlock. See Stephen Cleary's blog post on the subject of an async deadlock. Stephen's blog is an absolute gold mine of information on aysnc and await.

Either use the synchronous api all the way from your controller down into the Azure Storage SDK or use the asynchronous one (with async and await). Don't mix and match - that's a source of errors and also pointless. Aysnc is useful when there's something else the thread could be doing. E.g. in a web app that's serving requests. If the web app is blocked by .Result or Wait then there is no benefit and you should use the synchronous API.

Answer 2

private async Task<IEnumerable<Data>> GetDataAsync()
{
    //There are multiple blob address where the data is held. The code creates, in parallel multiple tasks for each address.
    //It returns tasks that will be run in Async pattern
    var tasks = multipleBlobAddress.AsParallel.Select(blobAddress =>{
            Task<IEnumerable<Data>> task = GetDataFromBlobsAsync(blobAddress);
            return task;
    });

Firstly, the purpose of AsParallel is to split work between multiple threads; the purpose of async/await is to avoid the need for multiple threads when handling things such as I/O. So combining AsParallel with async/await is a code smell.

Secondly, what's multipleBlobAddress? A field of the class? In my opinion it should be an argument to the method rather than a hidden parameter.

Thirdly, that lambda is far more complicated than it needs to be. The code I've quoted above could be

private async Task<IEnumerable<Data>> GetDataAsync(IEnumerable<string> blobAddresses)
{
    var tasks = blobAddresses.Select(GetDataFromBlobsAsync);

---

    //Awaits all tasks to complete
    var completedTasks = await Task.WhenAll<IEnumerable<Data>>(tasks);
    //Selects all the tasks and returns them. Each tasks has data.
    return completedTasks.SelectMany(t => t);
}

This will work, but it launches all of the downloads simultaneously. You'll probably find (and I speak from experience here) that you get better performance by running no more than n simultanous downloads for some value of n between 4 and 16. Writing a WhenAllThrottled<T extends Task>(IEnumerable<T>, int) method which uses WhenAny to launch a new task when one completes is a good exercise in async/await programming. Note that the IEnumerable<T> has to be lazy for it to actually work as intended.

---

private Task<IEnumerable<Data>> GetDataFromBlobsAsync(string address)
{
    //Opens the blob and reads from it
    using (var blobStream = blobService.OpenRead(address))
    {
        //Deserialises the data read from the blob
        data = service.DeserialiseData(blobStream);
    }

    return Task.FromResult<IEnumerable<Data>>(data);
}

This is quite alarming. What is data? It seems to be using a field to pass a value whose scope is a single method. If the tasks are executing in separate threads then this is a definite bug, because there's no synchronisation and one thread could overwrite data between another thread's assignment and invocation of Task.FromResult<T>. Without AsParallel it's probably safe, but it's definitely a code smell and risks becoming a race condition if the class is refactored.

I should also add that the name is confusing. If it's just getting data from one blob, why is it not GetDataFromBlobAsync?

Answer 3

To answer the questions first,

1. Running Parallelism is for better efficiency and speed when executing the same code repeatedly.
2. You can get threading wrong for not watching out for common threading issues. Msdn link has some good General Recommendations.
3. Not really, async and await is not the same as Parallelism as you might be thinking, there is an excellent course on Pluralsight which explained what it was and how it was properly implemented. - I'm not sure if I can provide a link can a moderator let me know?

Difference between the two is that the Parallelism/Threading is executing the same code but with different parameters at the same time.

Async and Await is a mechanism to let the computer decide to stop working the current process, Await, as the response is not ready at that time. The computer can go and do something else. When the Async result is returned the computer will come back and continue on. Basically not to block on the main thread.

Ok so looking at your code, delete method1 and use Async and await all the way through the code.

I've tried to change your code, but as its not complete I can only attempt to fill in the gaps for you.

public async Task<IEnumerable<Data>> GetDataAsync()
{
    //There are multiple blob address where the data is held. The code creates, in parallel multiple tasks for each address.
    //It returns tasks that will be run in Async pattern
    var tasks = multipleBlobAddress.AsParallel.Select(async blobAddress =>
    {
        IEnumerable<Data> task = await GetDataFromBlobsAsync(blobAddress);
        return task;
    });

    //Awaits all tasks to complete
    var completedTasks = await Task.WhenAll((IEnumerable<Data>)(tasks));
    //Selects all the tasks and returns them. Each tasks has data.
    return completedTasks.SelectMany(t => t);
}

private async Task<Data> GetDataFromBlobsAsync(string address)
{
    //Opens the blob and reads from it
    using (var blobStream = await blobService.OpenRead(address)) // at the bottom there should be a Async method
    {
        //Deserialises the data read from the blob
        data = service.DeserialiseData(blobStream);
    }

    return data;
}