Change script timeouts to be in milliseconds (FoundatioFx#75)

* Change script timeouts to be in milliseconds * Add lock renewal in test
Seadoo2017 · Sep 24, 2021 · 23bdea3 · 23bdea3
1 parent dfc89bb
commit 23bdea3
Show file tree

Hide file tree

Showing 8 changed files with 55 additions and 17 deletions.
diff --git a/docker-compose.yml b/docker-compose.yml
@@ -2,7 +2,7 @@ version: '2'
 
 services:
   redis:
-    image: grokzen/redis-cluster:6.0.1
+    image: grokzen/redis-cluster:6.2.1
     environment:
       IP: '0.0.0.0'
       STANDALONE: 'true'

diff --git a/src/Foundatio.Redis/Cache/RedisCacheClient.cs b/src/Foundatio.Redis/Cache/RedisCacheClient.cs
@@ -307,7 +307,7 @@ public async Task<double> SetIfHigherAsync(string key, double value, TimeSpan? e
             await LoadScriptsAsync().AnyContext();
 
             if (expiresIn.HasValue) {
-                var result = await Database.ScriptEvaluateAsync(_setIfHigher, new { key = (RedisKey)key, value, expires = expiresIn.Value.TotalSeconds }).AnyContext();
+                var result = await Database.ScriptEvaluateAsync(_setIfHigher, new { key = (RedisKey)key, value, expires = (int)expiresIn.Value.TotalMilliseconds }).AnyContext();
                 return (double)result;
             } else {
                 var result = await Database.ScriptEvaluateAsync(_setIfHigher, new { key = (RedisKey)key, value, expires = RedisValue.EmptyString }).AnyContext();
@@ -322,7 +322,7 @@ public async Task<long> SetIfHigherAsync(string key, long value, TimeSpan? expir
             await LoadScriptsAsync().AnyContext();
 
             if (expiresIn.HasValue) {
-                var result = await Database.ScriptEvaluateAsync(_setIfHigher, new { key = (RedisKey)key, value, expires = expiresIn.Value.TotalSeconds }).AnyContext();
+                var result = await Database.ScriptEvaluateAsync(_setIfHigher, new { key = (RedisKey)key, value, expires = (int)expiresIn.Value.TotalMilliseconds }).AnyContext();
                 return (long)result;
             } else {
                 var result = await Database.ScriptEvaluateAsync(_setIfHigher, new { key = (RedisKey)key, value, expires = RedisValue.EmptyString }).AnyContext();
@@ -337,7 +337,7 @@ public async Task<double> SetIfLowerAsync(string key, double value, TimeSpan? ex
             await LoadScriptsAsync().AnyContext();
 
             if (expiresIn.HasValue) {
-                var result = await Database.ScriptEvaluateAsync(_setIfLower, new { key = (RedisKey)key, value, expires = expiresIn.Value.TotalSeconds }).AnyContext();
+                var result = await Database.ScriptEvaluateAsync(_setIfLower, new { key = (RedisKey)key, value, expires = (int)expiresIn.Value.TotalMilliseconds }).AnyContext();
                 return (double)result;
             } else {
                 var result = await Database.ScriptEvaluateAsync(_setIfLower, new { key = (RedisKey)key, value, expires = RedisValue.EmptyString }).AnyContext();
@@ -352,7 +352,7 @@ public async Task<long> SetIfLowerAsync(string key, long value, TimeSpan? expire
             await LoadScriptsAsync().AnyContext();
 
             if (expiresIn.HasValue) {
-                var result = await Database.ScriptEvaluateAsync(_setIfLower, new { key = (RedisKey)key, value, expires = expiresIn.Value.TotalSeconds }).AnyContext();
+                var result = await Database.ScriptEvaluateAsync(_setIfLower, new { key = (RedisKey)key, value, expires = (int)expiresIn.Value.TotalMilliseconds }).AnyContext();
                 return (long)result;
             } else {
                 var result = await Database.ScriptEvaluateAsync(_setIfLower, new { key = (RedisKey)key, value, expires = RedisValue.EmptyString }).AnyContext();
@@ -394,7 +394,7 @@ public async Task<bool> ReplaceIfEqualAsync<T>(string key, T value, T expected,
             var expectedValue = expected.ToRedisValue(_options.Serializer);
             RedisResult redisResult;
             if (expiresIn.HasValue)
-                redisResult = await Database.ScriptEvaluateAsync(_replaceIfEqual, new { key = (RedisKey)key, value = redisValue, expected = expectedValue, expires = expiresIn.Value.TotalSeconds }).AnyContext();
+                redisResult = await Database.ScriptEvaluateAsync(_replaceIfEqual, new { key = (RedisKey)key, value = redisValue, expected = expectedValue, expires = (int)expiresIn.Value.TotalMilliseconds }).AnyContext();
             else
                 redisResult = await Database.ScriptEvaluateAsync(_replaceIfEqual, new { key = (RedisKey)key, value = redisValue, expected = expectedValue, expires = "" }).AnyContext();
 
@@ -414,7 +414,7 @@ public async Task<double> IncrementAsync(string key, double amount = 1, TimeSpan
 
             if (expiresIn.HasValue) {
                 await LoadScriptsAsync().AnyContext();
-                var result = await Database.ScriptEvaluateAsync(_incrementWithExpire, new { key = (RedisKey)key, value = amount, expires = expiresIn.Value.TotalSeconds }).AnyContext();
+                var result = await Database.ScriptEvaluateAsync(_incrementWithExpire, new { key = (RedisKey)key, value = amount, expires = (int)expiresIn.Value.TotalMilliseconds }).AnyContext();
                 return (double)result;
             }
 
@@ -432,7 +432,7 @@ public async Task<long> IncrementAsync(string key, long amount = 1, TimeSpan? ex
 
             if (expiresIn.HasValue) {
                 await LoadScriptsAsync().AnyContext();
-                var result = await Database.ScriptEvaluateAsync(_incrementWithExpire, new { key = (RedisKey)key, value = amount, expires = expiresIn.Value.TotalSeconds }).AnyContext();
+                var result = await Database.ScriptEvaluateAsync(_incrementWithExpire, new { key = (RedisKey)key, value = amount, expires = (int)expiresIn.Value.TotalMilliseconds }).AnyContext();
                 return (long)result;
             }
 

diff --git a/src/Foundatio.Redis/Scripts/DequeueId.lua b/src/Foundatio.Redis/Scripts/DequeueId.lua
@@ -2,8 +2,8 @@
 if item then
     local dequeuedTimeKey = 'q:' .. @queueName .. ':' .. item .. ':dequeued';
     local renewedTimeKey = 'q:' .. @queueName .. ':' .. item .. ':renewed';
-    redis.call('SET', dequeuedTimeKey, @now, 'EX', @timeout);
-    redis.call('SET', renewedTimeKey, @now, 'EX', @timeout);
+    redis.call('SET', dequeuedTimeKey, @now, 'PX', @timeout);
+    redis.call('SET', renewedTimeKey, @now, 'PX', @timeout);
     return item;
 else
     return nil;

diff --git a/src/Foundatio.Redis/Scripts/IncrementWithExpire.lua b/src/Foundatio.Redis/Scripts/IncrementWithExpire.lua
@@ -1,13 +1,13 @@
 if math.modf(@value) == 0 then
   local v = redis.call('incrby', @key, @value)
   if (@expires ~= nil and @expires ~= '') then
-    redis.call('expire', @key, math.ceil(@expires))
+    redis.call('pexpire', @key, math.ceil(@expires))
   end
   return tonumber(v)
 else
   local v = redis.call('incrbyfloat', @key, @value)
   if (@expires ~= nil and @expires ~= '') then
-    redis.call('expire', @key, math.ceil(@expires))
+    redis.call('pexpire', @key, math.ceil(@expires))
   end
   return tonumber(v)
 end
diff --git a/src/Foundatio.Redis/Scripts/ReplaceIfEqual.lua b/src/Foundatio.Redis/Scripts/ReplaceIfEqual.lua
@@ -1,7 +1,7 @@
 local currentVal = redis.call('get', @key)
 if (currentVal == false or currentVal == @expected) then
   if (@expires ~= nil and @expires ~= '') then
-    return redis.call('set', @key, @value, 'EX', @expires) and 1 or 0
+    return redis.call('set', @key, @value, 'PX', @expires) and 1 or 0
   else
     return redis.call('set', @key, @value) and 1 or 0
   end

diff --git a/src/Foundatio.Redis/Scripts/SetIfHigher.lua b/src/Foundatio.Redis/Scripts/SetIfHigher.lua
@@ -3,7 +3,7 @@ if c then
   if tonumber(@value) > c then
     redis.call('set', @key, @value)
     if (@expires ~= nil and @expires ~= '') then
-      redis.call('expire', @key, math.ceil(@expires))
+      redis.call('pexpire', @key, math.ceil(@expires))
     end
     return tonumber(@value) - c
   else
@@ -12,7 +12,7 @@ if c then
 else
   redis.call('set', @key, @value)
   if (@expires ~= nil and @expires ~= '') then
-    redis.call('expire', @key, math.ceil(@expires))
+    redis.call('pexpire', @key, math.ceil(@expires))
   end
   return tonumber(@value)
 end
diff --git a/src/Foundatio.Redis/Scripts/SetIfLower.lua b/src/Foundatio.Redis/Scripts/SetIfLower.lua
@@ -3,7 +3,7 @@ if c then
   if tonumber(@value) < c then
     redis.call('set', @key, @value)
     if (@expires ~= nil and @expires ~= '') then
-      redis.call('expire', @key, math.ceil(@expires))
+      redis.call('pexpire', @key, math.ceil(@expires))
     end
     return c - tonumber(@value)
   else
@@ -12,7 +12,7 @@ if c then
 else
   redis.call('set', @key, @value)
   if (@expires ~= nil and @expires ~= '') then
-    redis.call('expire', @key, math.ceil(@expires))
+    redis.call('pexpire', @key, math.ceil(@expires))
   end
   return tonumber(@value)
 end
diff --git a/tests/Foundatio.Redis.Tests/Locks/RedisLockTests.cs b/tests/Foundatio.Redis.Tests/Locks/RedisLockTests.cs
@@ -8,6 +8,9 @@
 using Foundatio.Redis.Tests.Extensions;
 using Foundatio.Tests.Locks;
 using Foundatio.Xunit;
+using Microsoft.Extensions.Logging;
+using Foundatio.Utility;
+using System.Diagnostics;
 
 namespace Foundatio.Redis.Tests.Locks {
     public class RedisLockTests : LockTestBase, IDisposable {
@@ -39,6 +42,41 @@ public override Task LockWillTimeoutAsync() {
             return base.LockWillTimeoutAsync();
         }
 
+        [Fact]
+        public async Task LockWontTimeoutEarly() {
+            Log.SetLogLevel<InMemoryCacheClient>(LogLevel.Trace);
+            Log.SetLogLevel<CacheLockProvider>(LogLevel.Trace);
+            Log.SetLogLevel<ScheduledTimer>(LogLevel.Trace);
+
+            var locker = GetLockProvider();
+            if (locker == null)
+                return;
+
+            _logger.LogInformation("Acquiring lock #1");
+            var testLock = await locker.AcquireAsync("test", timeUntilExpires: TimeSpan.FromSeconds(1));
+            _logger.LogInformation(testLock != null ? "Acquired lock #1" : "Unable to acquire lock #1");
+            Assert.NotNull(testLock);
+
+            _logger.LogInformation("Acquiring lock #2");
+            var testLock2 = await locker.AcquireAsync("test", acquireTimeout: TimeSpan.FromMilliseconds(500));
+            Assert.Null(testLock2);
+
+            _logger.LogInformation("Renew lock #1");
+            await testLock.RenewAsync(timeUntilExpires: TimeSpan.FromSeconds(1));
+
+            _logger.LogInformation("Acquiring lock #3");
+            testLock = await locker.AcquireAsync("test", acquireTimeout: TimeSpan.FromMilliseconds(500));
+            Assert.Null(testLock);
+
+            var sw = Stopwatch.StartNew();
+            _logger.LogInformation("Acquiring lock #4");
+            testLock = await locker.AcquireAsync("test", acquireTimeout: TimeSpan.FromSeconds(5));
+            sw.Stop();
+            _logger.LogInformation(testLock != null ? "Acquired lock #3" : "Unable to acquire lock #4");
+            Assert.NotNull(testLock);
+            Assert.True(sw.ElapsedMilliseconds > 400);
+        }
+
         [RetryFact]
         public override Task WillThrottleCallsAsync() {
             return base.WillThrottleCallsAsync();