What is good C# coding style for catching SQLException and retrying

I'd change the exception handling to only retry on certain errors:

• 1204, 1205 deadlocks
• -2 timeout
• -1 connection broken

These are the basic "retryable" errors

catch (SqlException ex)
{
    if !(ex.Number == 1205 || ex.Number == 1204 || ... )
    {
        throw
    }
    retryCount++;
    if (retryCount > MAX_RETRY) throw;
}

Edit, I clean forgot about waits so you don't hammer the SQL box:

• Add a 500 ms wait on deadlock
• Add a 5 sec delay on timeout

Edit 2:

I'm a Developer DBA, don't do much C#. My answer was to correct exception processing for the calls...

My enum of retryables for sql looks like this:

SqlConnectionBroken = -1,
SqlTimeout = -2,
SqlOutOfMemory = 701,
SqlOutOfLocks = 1204,
SqlDeadlockVictim = 1205,
SqlLockRequestTimeout = 1222,
SqlTimeoutWaitingForMemoryResource = 8645,
SqlLowMemoryCondition = 8651,
SqlWordbreakerTimeout = 30053

It's not good style, but sometimes you have to do it, because you simply can't change existing code and have to deal with it.

I am using the following generic method for this scenario. Note the PreserveStackTrace() method, which can sometimes be very helpful in a re-throw scenario.

public static void RetryBeforeThrow<T>(Action action, int retries, int timeout) where T : Exception
{
    if (action == null)
        throw new ArgumentNullException("action", string.Format("Argument '{0}' cannot be null.", "action"));

    int tries = 1;

    do
    {
        try
        {
            action();
            return;
        }
        catch (T ex)
        {
            if (retries <= 0)
            {
                PreserveStackTrace(ex);
                throw;
            }

            Thread.Sleep(timeout);
        }
    }
    while (tries++ < retries);
}

/// <summary>
/// Sets a flag on an <see cref="T:System.Exception"/> so that all the stack trace information is preserved 
/// when the exception is re-thrown.
/// </summary>
/// <remarks>This is useful because "throw" removes information, such as the original stack frame.</remarks>
/// <see href="http://weblogs.asp.net/fmarguerie/archive/2008/01/02/rethrowing-exceptions-and-preserving-the-full-call-stack-trace.aspx"/>
public static void PreserveStackTrace(Exception ex)
{
    MethodInfo preserveStackTrace = typeof(Exception).GetMethod("InternalPreserveStackTrace", BindingFlags.Instance | BindingFlags.NonPublic);
    preserveStackTrace.Invoke(ex, null);
}

You would call it like that:

RetryBeforeThrow<SqlException>(() => MethodWhichFails(), 3, 100);

There is no good style for doing something like this. You'd be better off figuring out why the request fails the first time but succeeds the second time.

It seems possible that Sql Server has to initially compile an execution plan and then execute the query. So the first call fails because the combined times exceed your timeout property, and succeeds the second time because the execution plan is already compiled and saved.

I don't know how UsersDataContext works, but it may be the case that you have the option to Prepare the query before actually executing it.

Real Answer: If I had to do this, I would retry just once and not again, like this:

var results = new List<UserSummaryDto>();
using (var ctx = new 
    UsersDataContext(ConfigurationManager.ConnectionStrings[CONNECTION_STRING_KEY].ConnectionString))
{
        try
        {
            results = ctx.SearchPhoneList(value, maxRows)
                         .Select(user => user.ToDto())
                         .ToList();
            break;
        }
        catch (SqlException)
        {
            try
            {
                results = ctx.SearchPhoneList(value, maxRows)
                         .Select(user => user.ToDto())
                         .ToList();
                break;
            }
            catch (SqlException)
            {
                // set return value, or indicate failure to user however
            }
        }
    }
}

return results;

While I might trust you to not abuse the retry process, you'd be tempting your successor to increase the retry count as a quick fix.

I think annotating a method with an aspect specifying the retry count would result in more structured code, although it needs some infrastructure coding.

You can simply use SqlConnectionStringBuilder properties to sql connection retry.

var conBuilder = new SqlConnectionStringBuilder("Server=.;Database=xxxx;Trusted_Connection=True;MultipleActiveResultSets=true"); conBuilder.ConnectTimeout = 90; conBuilder.ConnectRetryInterval = 15; conBuilder.ConnectRetryCount = 6;

Note:- Required .Net 4.5 or later.