Increase max retry count default value to 6 so that we cover 1 full m…

…inute.
dotnet · Apr 28, 2017 · 4ed67eb · 4ed67eb
1 parent 28970aa
commit 4ed67eb
Show file tree

Hide file tree

Showing 7 changed files with 164 additions and 128 deletions.
diff --git a/src/EFCore.SqlServer/SqlServerRetryingExecutionStrategy.cs b/src/EFCore.SqlServer/SqlServerRetryingExecutionStrategy.cs
@@ -22,7 +22,7 @@ public class SqlServerRetryingExecutionStrategy : ExecutionStrategy
         /// </summary>
         /// <param name="context"> The context on which the operations will be invoked. </param>
         /// <remarks>
-        ///     The default retry limit is 5, which means that the total amount of time spent before failing is 26 seconds plus the random factor.
+        ///     The default retry limit is 6, which means that the total amount of time spent before failing is about a minute.
         /// </remarks>
         public SqlServerRetryingExecutionStrategy(
             [NotNull] DbContext context)

diff --git a/src/EFCore/Storage/ExecutionStrategy.cs b/src/EFCore/Storage/ExecutionStrategy.cs
@@ -19,7 +19,7 @@ public abstract class ExecutionStrategy : IExecutionStrategy
         /// <summary>
         ///     The default number of retry attempts.
         /// </summary>
-        protected static readonly int DefaultMaxRetryCount = 5;
+        protected static readonly int DefaultMaxRetryCount = 6;
 
         /// <summary>
         ///     The default maximum time delay between retries, must be nonnegative.
@@ -105,8 +105,8 @@ protected ExecutionStrategy(
         /// </summary>
         protected static bool Suspended
         {
-            get { return _suspended.Value ?? false; }
-            set { _suspended.Value = value; }
+            get => _suspended.Value ?? false;
+            set => _suspended.Value = value;
         }
 
         /// <summary>
@@ -344,9 +344,10 @@ protected virtual void OnFirstExecution()
         {
             if (Context?.Database.CurrentTransaction != null)
             {
-                throw new InvalidOperationException(CoreStrings.ExecutionStrategyExistingTransaction(
-                    GetType().Name,
-                    nameof(DbContext) + "." + nameof(DbContext.Database) + "." + nameof(DatabaseFacade.CreateExecutionStrategy) + "()"));
+                throw new InvalidOperationException(
+                    CoreStrings.ExecutionStrategyExistingTransaction(
+                        GetType().Name,
+                        nameof(DbContext) + "." + nameof(DbContext.Database) + "." + nameof(DatabaseFacade.CreateExecutionStrategy) + "()"));
             }
 
             ExceptionsEncountered.Clear();

diff --git a/test/EFCore.SqlServer.FunctionalTests/ExecutionStrategyTest.cs b/test/EFCore.SqlServer.FunctionalTests/ExecutionStrategyTest.cs
@@ -161,7 +161,7 @@ public void Verification_is_retried_using_same_retry_limit()
                 var connection = (TestSqlServerConnection)context.GetService<ISqlServerConnection>();
 
                 connection.ExecutionFailures.Enqueue(new bool?[] { true, null, true, true });
-                connection.CommitFailures.Enqueue(new bool?[] { true, true, true });
+                connection.CommitFailures.Enqueue(new bool?[] { true, true, true, true });
 
                 context.Products.Add(new Product());
                 Assert.Throws<RetryLimitExceededException>(() =>
@@ -172,8 +172,8 @@ public void Verification_is_retried_using_same_retry_limit()
                             context));
                 context.ChangeTracker.AcceptAllChanges();
 
-                Assert.Equal(6, connection.OpenCount);
-                Assert.Equal(6, connection.ExecutionCount);
+                Assert.Equal(7, connection.OpenCount);
+                Assert.Equal(7, connection.ExecutionCount);
             }
 
             using (var context = CreateContext())

diff --git a/test/EFCore.SqlServer.FunctionalTests/Utilities/TestSqlServerConnection.cs b/test/EFCore.SqlServer.FunctionalTests/Utilities/TestSqlServerConnection.cs
@@ -40,16 +40,16 @@ protected TestSqlServerConnection(ISqlServerConnection connection)
 
         public virtual int? CommandTimeout
         {
-            get { return _realConnection.CommandTimeout; }
-            set { _realConnection.CommandTimeout = value; }
+            get => _realConnection.CommandTimeout;
+            set => _realConnection.CommandTimeout = value;
         }
 
         public virtual IDbContextTransaction CurrentTransaction { get; private set; }
 
         public virtual IValueBufferCursor ActiveCursor
         {
-            get { return _realConnection.ActiveCursor; }
-            set { _realConnection.ActiveCursor = value; }
+            get => _realConnection.ActiveCursor;
+            set => _realConnection.ActiveCursor = value;
         }
 
         public virtual bool IsMultipleActiveResultSetsEnabled => _realConnection.IsMultipleActiveResultSetsEnabled;

diff --git a/test/EFCore.SqlServer.FunctionalTests/Utilities/TestSqlServerRetryingExecutionStrategy.cs b/test/EFCore.SqlServer.FunctionalTests/Utilities/TestSqlServerRetryingExecutionStrategy.cs
@@ -13,13 +13,14 @@ public class TestSqlServerRetryingExecutionStrategy : SqlServerRetryingExecution
         {
             -1, // Physical connection is not usable
             -2, // Timeout
-            42008, // Mirroring (Only when a database is deleted and another one is crated in fast succession)
+            42008, // Mirroring (Only when a database is deleted and another one is created in fast succession)
             42019 // CREATE DATABASE operation failed
         };
 
         public TestSqlServerRetryingExecutionStrategy()
-            : base(new DbContext(new DbContextOptionsBuilder().UseSqlServer(TestEnvironment.DefaultConnection).Options),
-                  DefaultMaxRetryCount, DefaultMaxDelay, _additionalErrorNumbers)
+            : base(
+                new DbContext(new DbContextOptionsBuilder().UseSqlServer(TestEnvironment.DefaultConnection).Options),
+                DefaultMaxRetryCount, DefaultMaxDelay, _additionalErrorNumbers)
         {
         }
 
@@ -45,8 +46,7 @@ protected override bool ShouldRetryOn(Exception exception)
                 return true;
             }
 
-            var sqlException = exception as SqlException;
-            if (sqlException != null)
+            if (exception is SqlException sqlException)
             {
                 var message = "Didn't retry on";
                 foreach (SqlError err in sqlException.Errors)
@@ -56,8 +56,7 @@ protected override bool ShouldRetryOn(Exception exception)
                 throw new InvalidOperationException(message, exception);
             }
 
-            var invalidOperationException = exception as InvalidOperationException;
-            if (invalidOperationException != null
+            if (exception is InvalidOperationException invalidOperationException
                 && invalidOperationException.Message == "Internal .Net Framework Data Provider error 6.")
             {
                 return true;
@@ -74,8 +73,8 @@ protected override bool ShouldRetryOn(Exception exception)
 
         public new static bool Suspended
         {
-            get { return ExecutionStrategy.Suspended; }
-            set { ExecutionStrategy.Suspended = value; }
+            get => ExecutionStrategy.Suspended;
+            set => ExecutionStrategy.Suspended = value;
         }
     }
 }
diff --git a/test/EFCore.SqlServer.Tests/Storage/SqlServerRetryingExecutionStrategyTests.cs b/test/EFCore.SqlServer.Tests/Storage/SqlServerRetryingExecutionStrategyTests.cs
@@ -30,7 +30,8 @@ public void GetNextDelay_returns_shorter_delay_for_InMemory_transient_errors()
                 TimeSpan.FromMilliseconds(1),
                 TimeSpan.FromMilliseconds(3),
                 TimeSpan.FromMilliseconds(7),
-                TimeSpan.FromMilliseconds(15)
+                TimeSpan.FromMilliseconds(15),
+                TimeSpan.FromMilliseconds(31)
             };
 
             Assert.Equal(expectedDelays.Count, delays.Count);