From 23bdea349350b4ac9cf73cd3f828af9d1d3534fd Mon Sep 17 00:00:00 2001 From: "Eric J. Smith" Date: Fri, 24 Sep 2021 14:00:23 -0500 Subject: [PATCH] Change script timeouts to be in milliseconds (#75) * Change script timeouts to be in milliseconds * Add lock renewal in test --- docker-compose.yml | 2 +- src/Foundatio.Redis/Cache/RedisCacheClient.cs | 14 +++---- src/Foundatio.Redis/Scripts/DequeueId.lua | 4 +- .../Scripts/IncrementWithExpire.lua | 4 +- .../Scripts/ReplaceIfEqual.lua | 2 +- src/Foundatio.Redis/Scripts/SetIfHigher.lua | 4 +- src/Foundatio.Redis/Scripts/SetIfLower.lua | 4 +- .../Locks/RedisLockTests.cs | 38 +++++++++++++++++++ 8 files changed, 55 insertions(+), 17 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 862d1975..a86c2e15 100644 --- 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 index 6be3a67b..5b5b84c2 100644 --- a/src/Foundatio.Redis/Cache/RedisCacheClient.cs +++ b/src/Foundatio.Redis/Cache/RedisCacheClient.cs @@ -307,7 +307,7 @@ public async Task 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 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 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 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 ReplaceIfEqualAsync(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 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 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 index 89d7538d..9fdddd90 100644 --- 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 index 81f7fc95..2323a70e 100644 --- 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 \ No newline at end of file diff --git a/src/Foundatio.Redis/Scripts/ReplaceIfEqual.lua b/src/Foundatio.Redis/Scripts/ReplaceIfEqual.lua index 8525008e..b715eed2 100644 --- 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 index da3d1871..f3b4a9b1 100644 --- 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 \ No newline at end of file diff --git a/src/Foundatio.Redis/Scripts/SetIfLower.lua b/src/Foundatio.Redis/Scripts/SetIfLower.lua index 43d22c8f..5feb13d5 100644 --- 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 \ No newline at end of file diff --git a/tests/Foundatio.Redis.Tests/Locks/RedisLockTests.cs b/tests/Foundatio.Redis.Tests/Locks/RedisLockTests.cs index 17d4daac..58fcd7a2 100644 --- 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(LogLevel.Trace); + Log.SetLogLevel(LogLevel.Trace); + Log.SetLogLevel(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();