0

I have a large database and I access it using Entity Framework. I get all items as a List<Item>. When I process them in a loop:

List<Item> lstItems = UnitOfWork.Items.GetAllAsync(); // repository pattern and Unit Of Work patterns
foreach (Item item in lstItems)
{
  //do something with the item
}

What I need is if one of the items fail processing in the loop I want to be able to retry that action 3 times before ignoring it in this loop. Also I don't want the loop to brake, so no matter if an item processing is success or fail the loop should continue until the last item in the list.

Any idea of a pattern I can use for this purpose?

I was thinking about using try catch and if processing fails then in the catch section add it to a new List<Item> called processAgain and once the main list is finished processing then handle the processAgain list.

David Dury
  • 5,537
  • 12
  • 56
  • 94
  • 1
    About your suggestion of the catch section, exceptions have a performance penalty, and since your are in a loop, that might have a noticeable effect, so try to check if the item you have is valid, don't rely on exceptions, it should be your last resort. – sameh.q Aug 27 '14 at 05:44
  • Is the final order of the items important for you? – Stefan Over Aug 27 '14 at 05:49
  • @Herdo the order is not important, important is all items get processed and the ones who fail retry processing them 3 times before they get ignored for the current loop – David Dury Aug 27 '14 at 06:02
  • Creation - negative, structural - negative, behavioral - possibly? Is it a pattern you seek? The info I gather from your question is large database, retry, and I see a parallel-processing tag. What is the performance of your initial solution? Can you add a *success* variable to your *item* object and update to true, in a while loop exiting if all success or retry = 3? – nshah Aug 27 '14 at 06:23
  • You seem to use a async getting of the items (`GetAllAsync`). Did you think about processing them async? You could use the [Task.WaitAll](http://msdn.microsoft.com/en-us/library/dd270695%28v=vs.110%29.aspx) to process a bulk or even all data "at once". With this approach you neither have to wait for an item to complete -if it fails - to start the next one, nor do you need a second list and a second iteration process. – Stefan Over Aug 27 '14 at 06:26
  • @Herdo: That's true the GetAllAsync is async. I didn't thought about it but sound's a good idea. Can you describe a bit more how this can be achieved? – David Dury Aug 27 '14 at 06:39
  • I added an answer for you. – Stefan Over Aug 27 '14 at 08:09
  • @Herdo: Looks really good! Thanks a lot! – David Dury Aug 27 '14 at 08:17

5 Answers5

2

As you want to achive parallel processing, using a Task and async processing would be a good approach. Therefore, you need to define a method or expression that will be your Task/Action and includes a retry pattern internally:

public const int MAX_RETRY_COUNT = 3;

private void ProcessItemsAsync(Item item, int retryCount)
{        
    // Note: Exceptions thrown here will pop up
    // as AggregateException in Task.WaitAll()
    if (retryCount >= MAX_RETRY_COUNT)
        throw new InvalidOperationException(
            "The maximum amount of retries has been exceeded");

    retryCount++;
    // Either implement try-catch, or use conditional operator.
    try
    {
        // Do stuff with item
    }
    catch(Exception ex)
    {
        // Exception logging relevant? If not, just retry
        ProcessItemsAsync(item, retryCount);
    }
}

Once you defined your Task method, you can process a bulk of tasks at once:

public const int BULK_AMOUNT = 10;

private async void ProcessSqlData()
{
    List<Item> lstItems = await UnitOfWork.Items.GetAllAsync();
    for (var i = 0; i < lstItems.Count; i += BULK_AMOUNT)
    {
        // Running the process parallel with to many items
        // might slow down the whole process, so just take a bulk
        var bulk = lstItems.Skip(i).Take(BULK_AMOUNT).ToArray();
        var tasks = new Task[bulk.Length];
        for (var j = 0; j <= bulk.Length; j++)
        {
            // Create and start tasks, use ProcessItemsAsync as Action
            tasks[j] = Task.Factory.StartNew(() => ProcessItemsAsync(bulk[j], 0));
        }
        // Wait for the bulk to complete
        try
        {
            Task.WaitAll(tasks);
        }
        catch (AggregateException e)
        {
            Log.WriteLine(String.Format(
                "The maximum amount of retries has been exceeded in bulk #{0}. Error message: {1}",
                i,
                e.InnerException != null
                    ? e.InnerException.Message
                    : e.Message));
        }
    }
}

However, if you know that your computer running this has enough performance, you might increase the BULK_AMOUNT. You should test it, to find the optimal amount here.

Stefan Over
  • 5,851
  • 2
  • 35
  • 61
  • One question: what will be the benefit if we use instead of `Task.WaitAll(tasks)` -> `Task.WhenAll(tasks)` ? – David Dury Aug 27 '14 at 09:15
  • @DavidDury As Jon Skeet mentioned in [this](http://stackoverflow.com/a/6123432/2132796) post, `Task.WaitAll` blocks the executing thread, whereas `Task.WhenAll` returns an awaitable task. However, according to the [MSDN documentation](http://msdn.microsoft.com/en-us/library/hh160374%28v=vs.110%29.aspx) of `Task.WhenAll`, it doesn't throw a `AggregateException`, containing the real exceptions. You would need to take a look at the task result and check if the `Task` is in a faulted state. – Stefan Over Aug 27 '14 at 09:24
  • Is it necessary to throw the `AggregateException` in the `ProcessItemsAsync`? So basically the flow will be like this: There is a timer job that runs once a day and the job will pull all tasks from db, loop them and process them. If a specific task fails, that task should be retried 3 times. If fails 3 times then the timer job will ignore that task for today. When it will run tomorrow, then will process all of them again. And again if one task or the same fails then it should retry it 3 times before ignoring it. – David Dury Aug 27 '14 at 09:54
  • So basically I don't need the `AggregateException` to be thrown, and then I can use the `Task.WhenAll` instead and then the executing thread will not be blocked, and I will guess will result in a better performance? – David Dury Aug 27 '14 at 09:55
  • If you don't need the exception, then you can just return after `retryCount >= MAX_RETRY_COUNT`. The `AggregateException` is not manually thrown, it's thrown by `WaitAll`, if any tasks throws an exception. If your program has an UI, you should use `Task.WhenAll`. Otherwise it doesn't matter. There shouldn't be any performance differences. – Stefan Over Aug 27 '14 at 10:00
  • Yes of course, I meant this exception throwing `throw new InvalidOperationException` in `PorcessItemsAsync`. There is no UI in the program just a server side execution. – David Dury Aug 27 '14 at 10:08
  • So it doesn't really matter. But when it's a server application, you might want to log, that an item was skipped. Just as suggestion ;) You can for sure extend this for your required logic. – Stefan Over Aug 27 '14 at 10:13
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/60071/discussion-between-david-dury-and-herdo). – David Dury Aug 27 '14 at 10:20
1

I would suggest a recursive approach:

    static int errorCounter;

    static void Main(string[] args)
    {
        List<Item> lstItems = new List<Item>();
        foreach (var item in lstItems)
        {
            errorCounter = 0;
            bool succesful = CustomAction(item);
        }
    }

    static bool CustomAction(Item item)
    {
        try
        {
            //your custom actions with the item
        }
        catch (Exception ex)
        {

            if (errorCounter < 3)
            {
                errorCounter++;
                CustomAction(item);
            }
            else
            {
                return false;
            }
        }
        return true;
    }

EDIT: if for any reason you do not want to use a static counter, you can also initialize the counter in the Main method and pass it to CustomAction as a parameter

0

You can use Producer/Consumer pattern like here

Just create your processing logic (retry for three times) in Consume method before remove item from the queue

Community
  • 1
  • 1
Vladmir
  • 1,255
  • 9
  • 13
0

You can try to use recursion. Something like that:

List<Item> lstItems = UnitOfWork.Items.GetAllAsync(); // repository pattern and Unit Of Work patterns
foreach (Item item in lstItems)
{

    YourProcessingFunction(yourArgs, int count);
}

And in YourProcessingFunction(yourArgs, int count):

try
{
    if(count<3)
    {
       //Processing
       return someValue;
    }
    else
    {
        return;
    }
}
catch(Exception ex)
{
    return YourProcessingFunction(yourArgs, int count);
}
Gleb
  • 1,412
  • 1
  • 23
  • 55
0

there is no need of recursion as it will make your logic complex. you can use try catch block as follows

List<Item> lstItems = UnitOfWork.Items.GetAllAsync(); // repository pattern and Unit Of Work patterns

        foreach (Item item in lstItems)
        {
          try
          {
             //do something with the item
          }
          catch(Exception ex)
          {
             for(int i = 1; i <= 3; i++)
             {
                 // do something with the item
             }
             // to continue external loop
             continue;
          }

    }

i hope it will help :)

Zohaib Aslam
  • 585
  • 1
  • 4
  • 15