[Cache]: Added GetAllExpirationsAsync and SetAllExpirationsAsync. Also refactored tests and improved test coverage. #122
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Optimizes list retrieval by using SortedSetRangeByScoreAsync instead of SortedSetRangeByRankAsync, utilizing timestamp-based scores for efficient expiration management. Handles legacy sets by migrating them to sorted sets when encountering WRONGTYPE exceptions, ensuring compatibility and preventing data loss during the transition. Logs trace information when a sorted set is empty during expiration setting.
Updates the sorted set range query for retrieving cached lists to use Double.PositiveInfinity for the maximum score, ensuring all valid items are returned. This change enhances performance by avoiding unnecessary additions of 1 millisecond to the timestamp, streamlining the retrieval process and improving efficiency.
Implements GetAllExpiration and SetAllExpiration methods for retrieving and setting expirations for multiple cache keys. This significantly improves performance when dealing with numerous keys by leveraging Lua scripts and parallel processing, minimizing round trips to the Redis server. It also optimizes the RemoveAllAsync method to use parallel batch deletions and flushdb where possible.
Adds argument validation to prevent errors when null or empty keys are used with the Redis cache. Replaces multiple null or empty checks with ArgumentException.ThrowIfNullOrEmpty calls for consistency and conciseness. Also replaces `keys == null` with `keys is null`.
Ensures that the cache client throws an exception when attempting to set a value with a null or empty key. This prevents potential runtime errors and improves the reliability of the caching mechanism.
Adds validation for the input keys to GetAllAsync. Ensures that the input keys are not null or empty, and eliminates duplicate keys before proceeding with the cache retrieval operation. This enhancement improves the reliability and robustness of the method by preventing potential exceptions.
Optimizes key deletion within the Redis cache client, particularly for cluster configurations, by ensuring keys are processed in batches based on hash slots, thus minimizing Redis overload. Also, handles null or empty keys more robustly, preventing exceptions and improving overall stability.
Adds an ArgumentNullException check for the values parameter in the SetAllAsync method to prevent unexpected behavior when a null dictionary is passed. This improves consistency with all other callers
Documents that StackExchange.Redis handles pipelining internally, meaning the parallelism limits in place primarily affect client-side task creation.
- Use MSET for batch sets without expiration (Redis 1.0.1+) instead of pipelined individual SET commands, reducing round trips and Task overhead - Add MSETEX support for batch sets with expiration on Redis 8.4+ - Implement version detection with SupportsMsetexCommand() that: - Checks all connected primary servers for Redis 8.4+ (version 8.3.224) - Caches the result to avoid repeated version checks - Invalidates cache on connection restored (handles failover scenarios) - Includes debug logging for troubleshooting - Maintain cluster/sentinel support by grouping keys by hash slot - Fall back to pipelined individual SETs for expiration on Redis < 8.4 Note: MSET/MSETEX are atomic operations that replace existing values and remove any existing TTL, consistent with standard SET behavior.
Adds null checks to the TTL results from the Lua script evaluations. This ensures that the code gracefully handles unexpected responses from Redis, preventing potential errors when processing the TTL values.
niemyjski
added
enhancement
dependencies
Copilot finished reviewing on behalf of
niemyjski
November 25, 2025 01:45
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR adds GetAllExpirationsAsync and SetAllExpirationsAsync methods to support batch expiration operations in the Redis cache client. It also refactors tests with improved naming conventions, implements parallel processing for better performance, adds MSETEX support detection for Redis 8.4+, and enforces stricter argument validation.
Key changes:
- New batch expiration methods with dedicated Lua scripts
- Performance improvements through parallelization of batch operations
- Enhanced argument validation across all cache methods
- Comprehensive test coverage with descriptive naming
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Foundatio.Redis/Cache/RedisCacheClient.cs | Core implementation of new batch expiration methods, MSETEX support, parallelization, and argument validation |
| src/Foundatio.Redis/Scripts/GetAllExpiration.lua | Lua script for batch expiration retrieval |
| src/Foundatio.Redis/Scripts/SetAllExpiration.lua | Lua script for batch expiration setting |
| src/Foundatio.Redis/Foundatio.Redis.csproj | StackExchange.Redis version bump to 2.10.1 |
| tests/Foundatio.Redis.Tests/Caching/*.cs | Test method renaming for clarity and consistency |
| src/Foundatio.Redis/Extensions/EnumerableExtensions.cs | Removed unused Batch method |
| src/Foundatio.Redis/Extensions/TypeExtensions.cs | Removed extra blank line |
| src/Foundatio.Redis/Cache/RedisHybridCacheClientOptions.cs | Removed extra blank line |
Updates GitHubActionsTestLogger to version 3.0.1. Updates BenchmarkDotNet to version 0.15.6. These updates likely bring bug fixes, performance improvements, and new features to the testing and benchmarking processes.
Changes the exception type thrown when the script returns an unexpected number of results. Replaces ArgumentException with InvalidOperationException to better reflect the nature of the error.
Adds validation tests for GetAllExpirationAsync and SetAllExpirationAsync methods. Updates Foundatio dependency to pick up new test fixes.
Refactors cache key and expiration processing to prevent exceptions caused by null or empty keys. This change improves the robustness of the Redis cache client by adding explicit checks for null or empty keys, which can cause issues when setting expirations. It also optimizes key processing by pre-allocating lists when the input is a collection. This change avoids grouping by hash slot when no expirations are present to prevent unnecessary overhead.
Updates Foundatio and related package references to the 12.0.1-preview.0.26 version.
Comment on lines
+815
to
+835
| foreach (var hashSlotGroup in keyList.GroupBy(k => _options.ConnectionMultiplexer.HashSlot(k))) | ||
| { | ||
| var hashSlotKeys = hashSlotGroup.ToArray(); | ||
| var redisResult = await Database.ScriptEvaluateAsync(_getAllExpiration.Hash, hashSlotKeys).AnyContext(); | ||
| if (redisResult.IsNull) | ||
| continue; | ||
|
|
||
| // Lua script returns array of TTL values in milliseconds (in same order as keys) | ||
| // -2 = key doesn't exist, -1 = no expiration, positive = TTL in ms | ||
| long[] ttls = (long[])redisResult; | ||
| if (ttls is null || ttls.Length != hashSlotKeys.Length) | ||
| throw new InvalidOperationException($"Script returned {ttls?.Length ?? 0} results for {hashSlotKeys.Length} keys"); | ||
|
|
||
| for (int hashSlotIndex = 0; hashSlotIndex < hashSlotKeys.Length; hashSlotIndex++) | ||
| { | ||
| string key = hashSlotKeys[hashSlotIndex]; | ||
| long ttl = ttls[hashSlotIndex]; | ||
| if (ttl >= 0) // Only include keys with positive TTL (exclude non-existent and persistent) | ||
| result[key] = TimeSpan.FromMilliseconds(ttl); | ||
| } | ||
| } |
Comment on lines
+879
to
+888
| foreach (var hashSlotGroup in expirations.GroupBy(kvp => _options.ConnectionMultiplexer.HashSlot(kvp.Key))) | ||
| { | ||
| var hashSlotExpirations = hashSlotGroup.ToList(); | ||
| var keys = hashSlotExpirations.Select(kvp => (RedisKey)kvp.Key).ToArray(); | ||
| var values = hashSlotExpirations | ||
| .Select(kvp => (RedisValue)(kvp.Value.HasValue ? (long)kvp.Value.Value.TotalMilliseconds : -1)) | ||
| .ToArray(); | ||
|
|
||
| await Database.ScriptEvaluateAsync(_setAllExpiration.Hash, keys, values).AnyContext(); | ||
| } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#depends on FoundatioFx/Foundatio#426
There are one or two small breaking changes around argument validation where we are now enforcing you cannot pass in a null dictionary, and we are now asserting keys cannot be null or empty (white space is valid). This enforces consistency across our caching implementations (redis) as well as all cache method implementations that already took dictionaries and had null checks.