-2

I have a deviceList of more than 10k items and want to send data by calling another method.

I tried to use Parallel.Foreach but I'm not sure is this the correct way to do it.

I have published this webapp on azure, I have tested this for 100 it works fine but for 10k it got timeout issue. Is my implementation need any tuning , thanks

private List<Task> taskEventList = new List<Task>();
public async Task ProcessStart()
{
    string messageData = "{\"name\":\"DemoData\",\"no\":\"111\"}";
    RegistryManager registryManager;

    Parallel.ForEach(deviceList, async (device) =>
    {
        // get details for each device and use key to send message
        device = await registryManager.GetDeviceAsync(device.DeviceId);
        SendMessages(device.DeviceId, device.Key, messageData);
    });

    if (taskEventList.Count > 0)
    {
        await Task.WhenAll(taskEventList);
    }
}

private void SendMessages(string deviceId, string Key, string messageData)
{
    DeviceClient deviceClient = DeviceClient.Create(hostName, new DeviceAuthenticationWithRegistrySymmetricKey(deviceId, deviceKey), Microsoft.Azure.Devices.Client.TransportType.Mqtt);
    //created separate Task
    var taskEvents = Task.Run(() => ProcessMessages(deviceId, string messageData));
    taskEventList.Add(taskEvents);
}

private async Task ProcessMessages(string deviceId, string messageData)
{
    var startTime = DateTime.UtcNow;
    while (DateTime.UtcNow - startTime < TimeSpan.FromMinutes(15))
    {
        await deviceClient.SendEventAsync(messageData);
    }
}
Uwe Keim
  • 39,551
  • 56
  • 175
  • 291
Neo
  • 15,491
  • 59
  • 215
  • 405
  • 1
    Please format your code in a sensible way so we can read it – Joe Phillips Mar 15 '19 at 17:21
  • @JoePhillips updated it in sensible way :) pls have a look – Neo Mar 15 '19 at 17:26
  • That code will not build. The messageData string needs to have double quotes escaped. Also your formatting is still kind of awkward. Some places you're intending with 2 spaces, some with 8. It's really just difficult to put energy into reading this – Joe Phillips Mar 15 '19 at 17:27
  • I got your point let me update it again – Neo Mar 15 '19 at 17:28
  • 3
    Note that `Parallel.ForEach` is not intended for async IO tasks, it's meant for CPU bound operations. To spin up parallel IO tasks use `Select` and `Task.WhenAll` – JSteward Mar 15 '19 at 17:32
  • 1
    SemaphoreSlim is the throttling mechanism you will want to use here rather than Parallel.Foreach – Joe Phillips Mar 15 '19 at 17:34
  • thanks for your time can can one post it in answer with a sample code snippet , it will be helpful to me to implement :) – Neo Mar 15 '19 at 17:37
  • do you want me to use normal Foreach instead if Parallel.ForEach? – Neo Mar 15 '19 at 17:38

1 Answers1

5

There's definitely a race condition, at least. Parallel is only for synchronous code, not asynchronous.

As far as I can see, you don't need Parallel or Task.Run (which are both antipatterns for ASP.NET services):

public async Task ProcessStart()
{
  string messageData = "{\"name\":\"DemoData\",\"no\":\"111\"}";
  RegistryManager registryManager;

  var tasks = deviceList.Select(async device =>
  {
    // get details for each device and use key to send message
    device = await registryManager.GetDeviceAsync(device.DeviceId);
    await SendMessagesAsync(device.DeviceId, device.Key, messageData);
  }).ToList();

  await Task.WhenAll(tasks);
}

private async Task SendMessagesAsync(string deviceId, string Key, string messageData)
{
  DeviceClient deviceClient = DeviceClient.Create(hostName, new DeviceAuthenticationWithRegistrySymmetricKey(deviceId, deviceKey), Microsoft.Azure.Devices.Client.TransportType.Mqtt);
  await ProcessMessagesAsync(deviceId, string messageData);
}

private async Task ProcessMessagesAsync(string deviceId, string messageData)
{
  var startTime = DateTime.UtcNow;
  while (DateTime.UtcNow - startTime < TimeSpan.FromMinutes(15))
  {
    await deviceClient.SendEventAsync(messageData);
  }
}

for 10k it got timeout issue.

15 minutes is a long time for an HTTP request. I think it would be worthwhile to take a step back and see if there's a better way to architect the whole system.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • thanks a lot for answer , yes 15 minutes is a long time for an HTTP request but how to tackle this scenario ? I need to execute same code for 30 and 60 minutes as well :( can you help a little more in that context ? thanks in advance – Neo Mar 17 '19 at 07:27
  • 1
    @Neo One common approach is to have the HTTP request enter the work to be done in a (persistent) queue and return the ID of that work. Then there's a separate background process that processes the work in that queue. Meanwhile, the front end will poll a (different) HTTP endpoint to check the status of that ID so it knows when it's done and whether it has any errors. – Stephen Cleary Mar 17 '19 at 08:47
  • so you mean to say i can use azure function in the backend which will do update operation continuous for 15 minutes.... – Neo Mar 17 '19 at 16:18
  • Yes, an Azure Function is a good choice for background process. – Stephen Cleary Mar 17 '19 at 21:01
  • error was related with DeviceClient only getting exception ` {"I/O error occurred."}` i will post details another question mark this as answer. – Neo Mar 20 '19 at 10:12