Skip to content

Commit 420966e

Browse files
niemyjskiniemyjski
committed
Improves Redis cache expiration handling
Ensures proper handling of cache expiration by checking for values less than the minimum allowed expiration. Negative expiration values now delete keys immediately. Adds a lock to the shared connection.
1 parent ea94f49 commit 420966e

3 files changed

Lines changed: 29 additions & 17 deletions

File tree

‎src/Foundatio.Redis/Cache/RedisCacheClient.cs‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ public async Task<long> ListAddAsync<T>(string key, IEnumerable<T> values, TimeS
374374
ArgumentException.ThrowIfNullOrEmpty(key);
375375
ArgumentNullException.ThrowIfNull(values);
376376

377-
if (expiresIn is { Ticks: <= 0 })
377+
if (expiresIn < CacheClientExtensions.MinimumExpiration)
378378
{
379379
await ListRemoveAsync(key, values).AnyContext();
380380
return 0;
@@ -502,7 +502,7 @@ public async Task<double> SetIfHigherAsync(string key, double value, TimeSpan? e
502502
{
503503
ArgumentException.ThrowIfNullOrEmpty(key);
504504

505-
if (expiresIn is { Ticks: <= 0 })
505+
if (expiresIn < CacheClientExtensions.MinimumExpiration)
506506
{
507507
await RemoveAsync(key).AnyContext();
508508
return 0;
@@ -520,7 +520,7 @@ public async Task<long> SetIfHigherAsync(string key, long value, TimeSpan? expir
520520
{
521521
ArgumentException.ThrowIfNullOrEmpty(key);
522522

523-
if (expiresIn is { Ticks: <= 0 })
523+
if (expiresIn < CacheClientExtensions.MinimumExpiration)
524524
{
525525
await RemoveAsync(key).AnyContext();
526526
return 0;
@@ -538,7 +538,7 @@ public async Task<double> SetIfLowerAsync(string key, double value, TimeSpan? ex
538538
{
539539
ArgumentException.ThrowIfNullOrEmpty(key);
540540

541-
if (expiresIn is { Ticks: <= 0 })
541+
if (expiresIn < CacheClientExtensions.MinimumExpiration)
542542
{
543543
await RemoveAsync(key).AnyContext();
544544
return 0;
@@ -556,7 +556,7 @@ public async Task<long> SetIfLowerAsync(string key, long value, TimeSpan? expire
556556
{
557557
ArgumentException.ThrowIfNullOrEmpty(key);
558558

559-
if (expiresIn is { Ticks: <= 0 })
559+
if (expiresIn < CacheClientExtensions.MinimumExpiration)
560560
{
561561
await RemoveAsync(key).AnyContext();
562562
return 0;
@@ -572,7 +572,7 @@ public async Task<long> SetIfLowerAsync(string key, long value, TimeSpan? expire
572572

573573
private async Task<bool> InternalSetAsync<T>(string key, T value, TimeSpan? expiresIn = null, When when = When.Always, CommandFlags flags = CommandFlags.None)
574574
{
575-
if (expiresIn?.Ticks <= 0)
575+
if (expiresIn < CacheClientExtensions.MinimumExpiration)
576576
{
577577
_logger.LogTrace("Removing expired key: {Key}", key);
578578
await RemoveAsync(key).AnyContext();
@@ -592,7 +592,7 @@ public async Task<int> SetAllAsync<T>(IDictionary<string, T> values, TimeSpan? e
592592
if (values.Count is 0)
593593
return 0;
594594

595-
if (expiresIn?.Ticks <= 0)
595+
if (expiresIn < CacheClientExtensions.MinimumExpiration)
596596
{
597597
_logger.LogTrace("Removing expired keys: {Keys}", values.Keys);
598598
await RemoveAllAsync(values.Keys).AnyContext();
@@ -736,7 +736,7 @@ public async Task<bool> ReplaceIfEqualAsync<T>(string key, T value, T expected,
736736
{
737737
ArgumentException.ThrowIfNullOrEmpty(key);
738738

739-
if (expiresIn is { Ticks: <= 0 })
739+
if (expiresIn < CacheClientExtensions.MinimumExpiration)
740740
{
741741
_logger.LogTrace("Removing expired key: {Key}", key);
742742
await RemoveAsync(key).AnyContext();
@@ -761,7 +761,7 @@ public async Task<double> IncrementAsync(string key, double amount = 1, TimeSpan
761761
{
762762
ArgumentException.ThrowIfNullOrEmpty(key);
763763

764-
if (expiresIn is { Ticks: <= 0 })
764+
if (expiresIn < CacheClientExtensions.MinimumExpiration)
765765
{
766766
_logger.LogTrace("Removing expired key: {Key}", key);
767767
await RemoveAsync(key).AnyContext();
@@ -781,7 +781,7 @@ public async Task<long> IncrementAsync(string key, long amount = 1, TimeSpan? ex
781781
{
782782
ArgumentException.ThrowIfNullOrEmpty(key);
783783

784-
if (expiresIn is { Ticks: <= 0 })
784+
if (expiresIn < CacheClientExtensions.MinimumExpiration)
785785
{
786786
_logger.LogTrace("Removing expired key: {Key}", key);
787787
await RemoveAsync(key).AnyContext();
@@ -819,6 +819,9 @@ public Task SetExpirationAsync(string key, TimeSpan expiresIn)
819819
if (expiresIn == TimeSpan.MaxValue)
820820
return Database.KeyPersistAsync(key);
821821

822+
if (expiresIn < CacheClientExtensions.MinimumExpiration)
823+
return Database.KeyDeleteAsync(key);
824+
822825
return Database.KeyExpireAsync(key, expiresIn);
823826
}
824827

@@ -926,7 +929,7 @@ public async Task SetAllExpirationAsync(IDictionary<string, TimeSpan?> expiratio
926929
var hashSlotExpirations = hashSlotGroup.ToList();
927930
var keys = hashSlotExpirations.Select(kvp => (RedisKey)kvp.Key).ToArray();
928931
var values = hashSlotExpirations
929-
.Select(kvp => (RedisValue)(kvp.Value.HasValue ? (long)kvp.Value.Value.TotalMilliseconds : -1))
932+
.Select(kvp => (RedisValue)(GetExpirationMilliseconds(kvp.Value) ?? -1))
930933
.ToArray();
931934

932935
await Database.ScriptEvaluateAsync(_setAllExpiration.Hash, keys, values).AnyContext();
@@ -936,7 +939,7 @@ public async Task SetAllExpirationAsync(IDictionary<string, TimeSpan?> expiratio
936939
{
937940
var keys = expirations.Select(kvp => (RedisKey)kvp.Key).ToArray();
938941
var values = expirations
939-
.Select(kvp => (RedisValue)(kvp.Value.HasValue ? (long)kvp.Value.Value.TotalMilliseconds : -1))
942+
.Select(kvp => (RedisValue)(GetExpirationMilliseconds(kvp.Value) ?? -1))
940943
.ToArray();
941944

942945
await Database.ScriptEvaluateAsync(_setAllExpiration.Hash, keys, values).AnyContext();

‎src/Foundatio.Redis/Scripts/SetAllExpiration.lua‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
-- Set expiration times for multiple keys using Redis PEXPIRE and PERSIST commands.
22
-- KEYS: All keys to set expiration for
33
-- ARGV: TTL values in milliseconds corresponding to each key in KEYS
4-
-- -1 or 0 = Remove expiration (persist key indefinitely)
4+
-- Negative = Remove expiration (persist key indefinitely)
5+
-- 0 = Delete key immediately (PEXPIRE 0 causes Redis to delete the key)
56
-- Positive integer = Set expiration to this many milliseconds
67
--
78
-- Uses PEXPIRE for setting TTL (https://redis.io/docs/latest/commands/pexpire/)
@@ -10,7 +11,7 @@
1011
for i = 1, #KEYS do
1112
local ttl = tonumber(ARGV[i])
1213

13-
if ttl == nil or ttl <= 0 then
14+
if ttl == nil or ttl < 0 then
1415
redis.call('persist', KEYS[i])
1516
else
1617
redis.call('pexpire', KEYS[i], math.ceil(ttl))

‎tests/Foundatio.Redis.Tests/SharedConnection.cs‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ namespace Foundatio.Redis.Tests;
77

88
public static class SharedConnection
99
{
10+
private static readonly object _lock = new();
1011
private static ConnectionMultiplexer _muxer;
1112

1213
public static ConnectionMultiplexer GetMuxer(ILoggerFactory loggerFactory)
@@ -15,9 +16,16 @@ public static ConnectionMultiplexer GetMuxer(ILoggerFactory loggerFactory)
1516
if (String.IsNullOrEmpty(connectionString))
1617
return null;
1718

18-
if (_muxer == null)
19-
_muxer = ConnectionMultiplexer.Connect(connectionString, o => o.LoggerFactory = loggerFactory);
19+
if (_muxer is not null)
20+
return _muxer;
21+
22+
lock (_lock)
23+
{
24+
if (_muxer is not null)
25+
return _muxer;
2026

21-
return _muxer;
27+
_muxer = ConnectionMultiplexer.Connect(connectionString, o => o.LoggerFactory = loggerFactory);
28+
return _muxer;
29+
}
2230
}
2331
}

0 commit comments

Comments
 (0)