Improve retry logic

Author: jeffrymorris

Couchbase.AspNet.UnitTests/Couchbase.AspNet.UnitTests.csproj

@@ -68,7 +68,8 @@
   <ItemGroup>
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="ProviderHelperTests.cs" />
-    <Compile Include="SessionStateItemTests.cs" />
+    <Compile Include="SessionState\SessionStateItemTests.cs" />
+    <Compile Include="SessionState\CouchbaseSessionStateProviderTests.cs" />
   </ItemGroup>
   <ItemGroup>
     <None Include="app.config">

Couchbase.AspNet.UnitTests/SessionState/CouchbaseSessionStateProviderTests.cs

@@ -0,0 +1,64 @@
+using System;
+using System.Web;
+using System.Web.SessionState;
+using Couchbase.AspNet.SessionState;
+using Couchbase.Core;
+using Couchbase.IO;
+using Moq;
+using NUnit.Framework;
+
+namespace Couchbase.AspNet.UnitTests.SessionState
+{
+    [TestFixture()]
+    public class CouchbaseSessionStateProviderTests
+    {
+        [Test()]
+        public void GetSessionStoreItem_WhenKeyNotFound_ReturnsNull()
+        {
+            //arrange
+            var result = new Mock<IOperationResult<byte[]>>();
+            result.Setup(x => x.Status).Returns(ResponseStatus.KeyNotFound);
+            result.Setup(x => x.Value).Returns(new byte[0]);
+
+            var bucket = new Mock<IBucket>();
+            bucket.Setup(x => x.Get<byte[]>(It.IsAny<string>())).Returns(result.Object);         
+
+            bool locked;
+            TimeSpan lockAge;
+            object lockId;
+            SessionStateActions actions;
+
+            //Act
+            var sessionStateItem = CouchbaseSessionStateProvider.GetSessionStoreItem(
+               bucket.Object, null, false, "thekey", out locked, out lockAge, out lockId, out actions);
+
+            //Assert
+            Assert.IsNull(sessionStateItem);
+        }
+
+        [Test()]
+        public void SetAndReleaseItemExclusive_WhenKeyNotFound_ReturnsNull()
+        {
+            //arrange
+            var result = new Mock<IOperationResult<byte[]>>();
+            result.Setup(x => x.Status).Returns(ResponseStatus.KeyNotFound);
+            result.Setup(x => x.Value).Returns(new byte[0]);
+
+            var bucket = new Mock<IBucket>();
+            bucket.Setup(x => x.Get<byte[]>(It.IsAny<string>())).Returns(result.Object);
+
+            var provider = new CouchbaseSessionStateProvider(new Mock<ICluster>().Object, bucket.Object);
+
+            bool locked;
+            TimeSpan lockAge;
+            object lockId = 10ul;
+            SessionStateActions actions;
+
+            //Act
+            provider.SetAndReleaseItemExclusive(null, "thekey", new SessionStateStoreData(new SessionStateItemCollection(), new HttpStaticObjectsCollection(), 10), lockId, false);
+
+
+
+        }
+    }
+}

Couchbase.AspNet.UnitTests/SessionStateItemTests.cs renamed to Couchbase.AspNet.UnitTests/SessionState/SessionStateItemTests.cs

@@ -4,7 +4,7 @@
 using Moq;
 using NUnit.Framework;
 
-namespace Couchbase.AspNet.UnitTests
+namespace Couchbase.AspNet.UnitTests.SessionState
 {
     [TestFixture]
     public class SessionStateItemTests

Couchbase.AspNet/Couchbase.AspNet.csproj

@@ -74,7 +74,7 @@
     <Compile Include="SessionState\CouchbaseSessionStateProvider.cs" />
     <Compile Include="ICouchbaseBucketFactory.cs" />
     <Compile Include="ProviderHelper.cs" />
-    <Compile Include="SessionState\SessionStateItemcs.cs" />
+    <Compile Include="SessionState\SessionStateItem.cs" />
     <Compile Include="Web\CouchbaseHttpApplication.cs" />
   </ItemGroup>
   <ItemGroup>

Couchbase.AspNet/SessionState/CouchbaseSessionStateProvider.cs

@@ -3,14 +3,39 @@
 using System.Web.SessionState;
 using System.Web;
 using Couchbase.Core;
+using Couchbase.IO;
 
 namespace Couchbase.AspNet.SessionState
 {
+    /// <summary>
+    /// A <see cref="SessionStateStoreProviderBase"/> implementation which uses Couchbase Server as the backing store.
+    /// </summary>
     public class CouchbaseSessionStateProvider : SessionStateStoreProviderBase
     {
         private ICluster _cluster;
         private IBucket _bucket;
         private static bool _exclusiveAccess;
+        private int _maxRetryCount = 5;
+
+        /// <summary>
+        /// Required default ctor for ASP.NET
+        /// </summary>
+        public CouchbaseSessionStateProvider()
+        {
+        }
+
+        /// <summary>
+        /// For unit testing only.
+        /// </summary>
+        /// <param name="cluster"></param>
+        /// <param name="bucket"></param>
+        public CouchbaseSessionStateProvider(
+            ICluster cluster,
+            IBucket bucket)
+        {
+            _cluster = cluster;
+            _bucket = bucket;
+        }
 
         /// <summary>
         /// Defines the prefix for header data in the Couchbase bucket. Must be unique to ensure it does not conflict with 
@@ -28,7 +53,6 @@ public class CouchbaseSessionStateProvider : SessionStateStoreProviderBase
             (System.Web.Hosting.HostingEnvironment.SiteName ?? string.Empty).Replace(" ", "-") + "+" +
             System.Web.Hosting.HostingEnvironment.ApplicationVirtualPath + "data-";
 
-
         /// <summary>
         /// Function to initialize the provider
         /// </summary>
@@ -39,11 +63,16 @@ public override void Initialize(string name, NameValueCollection config)
             // Initialize the base class
             base.Initialize(name, config);
 
-            // Create our Cluster based off the CouchbaseConfigSection
-            _cluster = ProviderHelper.GetCluster(name, config);
-
-            // Create the bucket based off the name provided in the
-            _bucket = ProviderHelper.GetBucket(name, config, _cluster);
+            if (_cluster == null)
+            {
+                // Create our Cluster based off the CouchbaseConfigSection
+                _cluster = ProviderHelper.GetCluster(name, config);
+            }
+            if (_bucket == null)
+            {
+                // Create the bucket based off the name provided in the
+                _bucket = ProviderHelper.GetBucket(name, config, _cluster);
+            }
 
             // By default use exclusive session access. But allow it to be overridden in the config file
             var exclusive = ProviderHelper.GetAndRemove(config, "exclusiveAccess", false) ?? "true";
@@ -60,6 +89,12 @@ public override void Initialize(string name, NameValueCollection config)
             {
                 DataPrefix = dataPrefix;
             }
+            var maxRetryCount = ProviderHelper.GetAndRemove(config, "maxRetryCount", false);
+            var temp = 0;
+            if (int.TryParse(maxRetryCount, out temp))
+            {
+                _maxRetryCount = temp;
+            }
 
             // Make sure no extra attributes are included
             ProviderHelper.CheckForUnknownAttributes(config);
@@ -130,7 +165,8 @@ public override void CreateUninitializedItem(HttpContext context, string id, int
                 Timeout = timeout
             };
 
-            e.SaveAll(_bucket, id, false);
+            bool keyNotFound;
+            e.SaveAll(_bucket, id, false, out keyNotFound);
         }
 
         /// <summary>
@@ -227,14 +263,19 @@ public static SessionStateItem GetSessionStoreItem(
                     e.LockTime = DateTime.UtcNow;
                     e.Flag = SessionStateActions.None;
 
+                    ResponseStatus status;
                     // try to update the item in the store
-                    if (e.SaveHeader(bucket, id, _exclusiveAccess))
+                    if (e.SaveHeader(bucket, id, _exclusiveAccess, out status))
                     {
                         locked = true;
                         lockId = e.LockId;
 
                         return e;
                     }
+                    if (status == ResponseStatus.KeyNotFound)
+                    {
+                        break;
+                    }
 
                     // it has been modified between we loaded and tried to save it
                     e = SessionStateItem.Load(bucket, id, false);
@@ -267,6 +308,8 @@ public override void SetAndReleaseItemExclusive(
             object lockId,
             bool newItem)
         {
+            bool keyNotFound;
+            var retries = 0;
             SessionStateItem e;
             do {
                 if (!newItem)
@@ -298,7 +341,7 @@ public override void SetAndReleaseItemExclusive(
                 e.LockTime = DateTime.MinValue;
 
                 // Attempt to save with CAS and loop around if it fails
-            } while (!e.SaveAll(_bucket, id, _exclusiveAccess && !newItem));
+            } while (!e.SaveAll(_bucket, id, _exclusiveAccess && !newItem, out keyNotFound) && retries++ < _maxRetryCount && !keyNotFound);
         }
 
         /// <summary>
@@ -312,6 +355,8 @@ public override void ReleaseItemExclusive(
             string id,
             object lockId)
         {
+            ResponseStatus status;
+            var retries = 0;
             var tmp = (ulong)lockId;
             SessionStateItem e;
             do {
@@ -327,7 +372,7 @@ public override void ReleaseItemExclusive(
                 // Attempt to clear the lock for this item and loop around until we succeed
                 e.LockId = 0;
                 e.LockTime = DateTime.MinValue;
-            } while (!e.SaveHeader(_bucket, id, _exclusiveAccess));
+            } while (!e.SaveHeader(_bucket, id, _exclusiveAccess, out status) && retries < _maxRetryCount && status != ResponseStatus.KeyNotFound);
         }
 
         /// <summary>
@@ -361,6 +406,8 @@ public override void ResetItemTimeout(
             HttpContext context,
             string id)
         {
+            bool keyNotFound;
+            var retries = 0;
             SessionStateItem e;
             do {
                 // Load the item with CAS
@@ -371,7 +418,7 @@ public override void ResetItemTimeout(
                 }
 
                 // Try to save with CAS, and loop around until we succeed
-            } while (!e.SaveAll(_bucket, id, _exclusiveAccess));
+            } while (!e.SaveAll(_bucket, id, _exclusiveAccess, out keyNotFound) && retries < _maxRetryCount && !keyNotFound);
         }
 
         /// <summary>

Couchbase.AspNet/SessionState/SessionStateItemcs.cs renamed to Couchbase.AspNet/SessionState/SessionStateItem.cs

@@ -53,7 +53,8 @@ private void WriteHeader(
         public bool SaveHeader(
             IBucket bucket,
             string id,
-            bool useCas)
+            bool useCas,
+            out ResponseStatus status)
         {
             using (var ms = new MemoryStream())
             {
@@ -64,6 +65,8 @@ public bool SaveHeader(
                 var retval = useCas
                     ? bucket.Upsert(CouchbaseSessionStateProvider.HeaderPrefix + id, ms.ToArray(), HeadCas, ts)
                     : bucket.Upsert(CouchbaseSessionStateProvider.HeaderPrefix + id, ms.ToArray(), ts);
+
+                status = retval.Status;
                 return retval.Success;
             }
         }
@@ -74,11 +77,13 @@ public bool SaveHeader(
         /// <param name="bucket">Couchbase bucket to save to</param>
         /// <param name="id">Session ID</param>
         /// <param name="useCas">True to use a check and set, false to simply store it</param>
+        /// <param name="status">The <see cref="ResponseStatus"/> from the server.</param>
         /// <returns>True if the value was saved, false if not</returns>
         public bool SaveData(
             IBucket bucket,
             string id,
-            bool useCas)
+            bool useCas,
+            out ResponseStatus status)
         {
             var ts = TimeSpan.FromMinutes(Timeout);
             using (var ms = new MemoryStream())
@@ -91,6 +96,7 @@ public bool SaveData(
                     ? bucket.Upsert(CouchbaseSessionStateProvider.DataPrefix + id, ms.ToArray(), DataCas, ts)
                     : bucket.Upsert(CouchbaseSessionStateProvider.DataPrefix + id, ms.ToArray(), ts);
 
+                status = retval.Status;
                 return retval.Success;
             }
         }
@@ -101,13 +107,20 @@ public bool SaveData(
         /// <param name="bucket">Couchbase bucket to save to</param>
         /// <param name="id">Session ID</param>
         /// <param name="useCas">True to use a check and set, false to simply store it</param>
+        /// <param name="keyNotFound">True if <see cref="ResponseStatus.KeyNotFound"/> is returned for the body or the header.</param>
         /// <returns>True if the value was saved, false if not</returns>
         public bool SaveAll(
             IBucket bucket,
             string id,
-            bool useCas)
+            bool useCas,
+            out bool keyNotFound)
         {
-            return SaveData(bucket, id, useCas) && SaveHeader(bucket, id, useCas);
+            var dataStatus = ResponseStatus.None;
+            var headerStatus = ResponseStatus.None;
+
+            var failed = SaveData(bucket, id, useCas, out dataStatus) && SaveHeader(bucket, id, useCas, out headerStatus);
+            keyNotFound = dataStatus == ResponseStatus.KeyNotFound || headerStatus == ResponseStatus.KeyNotFound;
+            return failed;
         }
 
         /// <summary>