2

I have asp.net application. All business logic in business layer.

Here is the example of the method

public void DoSomething()
{
        PersonClass pc = new PersonClass();

        pc.CreatePerson();
        pc.AssignBasicTask();
        pc.ChangePersonsStatus();
        pc.CreateDefaultSettings();    
}

what happens once in a while, one of the sub method can timeout, so as a result the process can be incompleted.

what I think in this case to make sure all steps completed properly is

public void DoSomething()
    {
            PersonClass pc = new PersonClass();
            var error = null;

            error =  pc.CreatePerson();

            if(error != timeout exception)                  
             error = pc.AssignBasicTask();
            else
              return to step above

            if(error != timeout exception)
              error = pc.ChangePersonsStatus();
            else
              return to step above

            if(error != timeout exception)
              error = pc.CreateDefaultSettings();
            else
              return to step above    
    }

but it's just an idea, more then sure it's a proper way how to handle this.

  • 1
    You need up fix the issue with the timeout. Retry will only throw more traffic at it and increase the problem. Your probably missing a database index. – TheCodeKing Sep 20 '11 at 18:49
  • We have index and it happens quite rarely. Maybe once per 1000 transaction – AlexanderTheGreat Sep 20 '11 at 18:51
  • @TheCodeKing That assumes the timeout is because of a database or database problem; what if he's using a REST service or WCF service or something of that sort, timeouts in that case could occur for a multitude of reasons. – CodingGorilla Sep 20 '11 at 18:52
  • 2
    This doesn't make sense, really. As designed, your app will try as long as forever to execute the process. If you're determined to do this (and I think you'd be wise to reconsider), you should set the timeout to infinite. At least that way, the process has all the time you're willing to give it to run. – Daniel Pratt Sep 20 '11 at 18:57
  • If something times out, that doesn't necessarily mean that it doesn't complete successfully. What it means is that your call didn't find out whether it was successful before a specific period of time elapsed. Consider that possibility in code that keeps retrying on timeouts. – hatchet - done with SOverflow Sep 20 '11 at 19:09

3 Answers3

2

You have it pretty close to correct in your psuedo-code, and there a lot of ways to do this, but here is how I would do it:

PersonClass pc = new PersonClass();
while(true)
   if(pc.CreatePerson())
      break;

while(true)
   if(pc.AssignBasicTask())
      break;

This assumes that your methods return true to indicate success, false to indicate a timeoiut failure (and probably an exception for any other kind of failure). And while I didn't do it here, I would strongly recommend some sort of try counting to make sure it doesn't just loop forever and ever.

CodingGorilla
  • 19,612
  • 4
  • 45
  • 65
2

Of course, this can be done more or less elegantly, with different options for timing out or giving up - but an easy way to achieve what you want, would be to define a retry method which keeps retrying an action until it succeeds:

public static class RetryUtility 
{
    public T RetryUntilSuccess<T>(Func<T> action) 
    {
        while(true) 
        {
            try 
            {
                return action();
            }
            catch 
            {
                // Swallowing exceptions is BAD, BAD, BAD. You should AT LEAST log it.
            }
        }
    }

    public void RetryUntilSuccess(Action action) 
    {
        // Trick to allow a void method being passed in without duplicating the implementation.
        RetryUntilSuccess(() => { action(); return true; });
    }
}

Then do

    RetryUtility.RetryUntilSuccess(() => pc.CreatePerson());
    RetryUtility.RetryUntilSuccess(() => pc.AssignBasicTask());
    RetryUtility.RetryUntilSuccess(() => pc.ChangePersonsStatus());
    RetryUtility.RetryUntilSuccess(() => pc.CreateDefaultSettings());   

I must urge you to think about what to do if the method keeps failing, you could be creating an infinite loop - perhaps it should give up after N retries or back off with exponentially raising retry time - you will need to define that, since we cannot know enough about your problem domain to decide that.

driis
  • 161,458
  • 45
  • 265
  • 341
0

Use a TransactionScope for to make sure everything is executed as a unit. More info here: Implementing an Implicit Transaction using Transaction Scope

You should never retry a timed out operation infinitely, you may end up hanging the server or with an infinite loop or both. There should always be a threshold of how many retries is acceptable to attempt before quitting.

Sample:

using(TransactionScope scope = new TransactionScope())
{
   try
   {
      // Your code here

      // If no errors were thrown commit your transaction
      scope.Complete();          
   }
   catch
   {
      // Some error handling
   }
}
Dmitry Samuylov
  • 1,554
  • 2
  • 14
  • 37
  • 1
    It doesn't say anywhere in the question that a database is involved. – driis Sep 20 '11 at 18:56
  • 1
    @driis: http://stackoverflow.com/questions/2273419/c-system-transactions-transactionscope – ScottE Sep 20 '11 at 19:02
  • @ScottE, still databases is what most people would know to use it for. Also, when looking at the code in question, I am willing to bet that there is no custom implementations of `IEnlistmentNotification` in play. – driis Sep 20 '11 at 19:04
  • @driis Just because most people don't know to use it in a non-database scenario doesn't mean its not a valid solution. The whole point here is to provide some alternatives to the person asking the question. They can decide for themselves which one works for them. And who knows, someone might actually learn something! I learn something new every day reading questions and answers on stackoverflow myself. Cheers! – Dmitry Samuylov Sep 20 '11 at 19:11