diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b24feda..93623cb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -373,10 +373,26 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) `app:s:{authz_{orgid}_groups_1}` — Redis hashes from the first `{` to the next `}`, so the tag was the accidental `authz_{orgid` rather than the org the caller asked for. A key with a valid hash tag (non-empty content between the first `{` and the next `}`) is now prefixed and left as-is, so the - caller's tag alone picks the slot and multi-key commands land where the caller intended. Keys with - no braces, or with braces that do not form a valid tag, are wrapped exactly as before. This - relocates existing entries only for apps that run `ShardKeyEnabled: true` *and* put braces in - their keys. + caller's tag picks the slot and multi-key commands land where the caller intended. Non-empty keys with no + braces are wrapped exactly as before; keys whose braces form no valid tag were wrapped by this change too, and + are refused outright by *A cache key that cannot carry a hash tag is refused rather than wrapped* later in this + release. This relocates existing entries only for apps + that run `ShardKeyEnabled: true` *and* put braces in their keys. + +### Deprecated + +- **`CacheOptions.ShardKeyEnabled`.** It wraps a brace-free cache key in a `{...}` hash tag so the slot follows + the key rather than `AppShortName` and the differentiator. That changes *which* keys share a slot and nothing + else: the tag becomes the whole key, unique per key just as the untagged key was, so it spreads no better, and + it cannot make a multi-key batch land on one node. A key that already carries a valid `{tag}` is rendered + identically on either setting, so the flag makes no difference to batching in either direction; a key whose + braces form no valid tag is refused outright when it is set. What it does do is co-locate one key across every + cache that renders through `DefaultRedisKeyStrategyFactory`, and across apps with a different `AppShortName` — + neither of which anyone asked for. What does matter on a cluster is unconditional: a caller's own tag survives, + and a cross-slot batch is refused with a message naming the two keys that disagree. The property is still read, + because a deployment that set it would relocate every brace-free key if it stopped being honored; to batch + across keys on a cluster, give the keys a shared hash tag yourself. All of this assumes `AppShortName`, + `Separator` and the differentiator are brace-free, since braces there change which span Redis hashes. ### Removed diff --git a/docs/how-to/telemetry-and-strategies.md b/docs/how-to/telemetry-and-strategies.md index 5810a1e7..7c3b466c 100644 --- a/docs/how-to/telemetry-and-strategies.md +++ b/docs/how-to/telemetry-and-strategies.md @@ -329,7 +329,7 @@ When `NotifyShardedPubSub` is `true` on `RedisStreamsTopicOptions`, the stream k ### When to reach for these seams -Most consumers never override any of these seams. The built-in `DefaultRedisKeyStrategyFactory` covers `ICache` and `IHashCache` and handles shard-key routing automatically when `CacheOptions.ShardKeyEnabled` is `true`: the key is wrapped in a `{...}` hash tag so the slot follows the key alone, and a key that already carries a valid hash tag is left as it is. Reasons to override: +Most consumers never override any of these seams. The built-in `DefaultRedisKeyStrategyFactory` covers `ICache` and `IHashCache`, and `RedisSetCache` and the distributed adapter through its string-differentiator overload; it leaves a key that already carries a valid hash tag alone. (`CacheOptions.ShardKeyEnabled` is deprecated — see [reference/settings.md](../reference/settings.md).) Reasons to override: - **Tenant/region shard routing** — you want Redis keys to encode tenant or region info as a Redis Cluster hash tag (`{tag}`) so related keys cluster to the same slot and cross-slot multi-key commands work correctly. - **Legacy key layout compatibility** — you are sharing a Redis instance with an existing system that has a fixed key schema you must match. Overriding the factory lets you produce keys that match the legacy layout without changing the cache call sites. diff --git a/docs/reference/settings.md b/docs/reference/settings.md index a8039fff..8f0aea28 100644 --- a/docs/reference/settings.md +++ b/docs/reference/settings.md @@ -20,7 +20,7 @@ Every binding-visible property on every shipped options class, with shipped defa | `Enabled` | `bool` | `true` | App-wide | Master on/off switch for the caching subsystem. | | `TelemetryEnabled` | `bool` | `true` | App-wide | Gates the `ICachingTelemetryProvider` seam; set to `false` to silence all cache metrics. | | `BroadcastEnabled` | `bool` | `true` | App-wide | Gates the `ITopicFactory` wiring; set to `false` to disable all invalidation broadcasts. | -| `ShardKeyEnabled` | `bool` | `false` | App-wide | Enable for Redis Cluster deployments that span multiple shards. Wraps the cache key in a `{...}` hash tag so only the key, not `AppShortName` or the keyspace, picks the slot. A key that already carries a valid hash tag is left as it is, so a caller that placed its own `{tag}` keeps the slot it chose. | +| `ShardKeyEnabled` | `bool` | `false` | App-wide | **Deprecated.** Wraps a brace-free cache key in a `{...}` hash tag so the key alone picks its slot. That changes which keys co-locate and nothing more: the tag is the whole key, so it spreads no better and cannot make a multi-key batch single-slot. A key that already carries a valid tag renders the same either way; one whose braces form no valid tag is refused. Keep braces out of `AppShortName`, `Separator` and the differentiator too, since braces there change which span Redis hashes. Flipping the flag rewrites every brace-free key, so leave it as your deployment has it. To batch across keys on a cluster, give the keys a shared hash tag yourself. | | `AuditEnabled` | `bool` | `true` | App-wide | Log writes whose serialized size exceeds `LargeValueThreshold` bytes. | | `DefaultCache` | `string` | `"InMemoryRedis"` | App-wide | Provider name resolved when no explicit provider is requested; values: `InMemory`, `Redis`, `InMemoryRedis`. | | `DefaultTopic` | `string` | `"RedisStreams"` | App-wide | Topic provider used when no explicit topic is requested; values: `RedisStreams`, `RedisPubSub`. | diff --git a/samples/UiPath.Caching.Sample/README.md b/samples/UiPath.Caching.Sample/README.md index f294f000..67481d77 100644 --- a/samples/UiPath.Caching.Sample/README.md +++ b/samples/UiPath.Caching.Sample/README.md @@ -92,7 +92,9 @@ Remove-Item Env:SampleAspNetCore__UseShardedRedis ``` Shard mode sets `Caching:ShardKeyEnabled=true` for both sample instances and -exposes Redis node ports `6379` through `6384`. With Redis Insight enabled, the +exposes Redis node ports `6379` through `6384`. That setting is +[deprecated](../../docs/reference/settings.md) and shard mode sets it only to +exercise the layout it produces. With Redis Insight enabled, the AppHost exposes Redis Insight on `http://localhost:8001` and `http://localhost:8002`. diff --git a/samples/UiPath.Caching.Sample/appsettings.all.json b/samples/UiPath.Caching.Sample/appsettings.all.json index c0e8db81..ad6cb5af 100644 --- a/samples/UiPath.Caching.Sample/appsettings.all.json +++ b/samples/UiPath.Caching.Sample/appsettings.all.json @@ -9,7 +9,7 @@ "TelemetryEnabled": true, // BroadcastEnabled: gate the ITopicFactory wiring "BroadcastEnabled": true, - // ShardKeyEnabled: enable for Redis Cluster with multiple shards + // ShardKeyEnabled: deprecated; changes which keys co-locate, nothing more "ShardKeyEnabled": false, // AuditEnabled: log writes above LargeValueThreshold "AuditEnabled": true, diff --git a/src/UiPath.Caching.Abstractions/CacheOptions.cs b/src/UiPath.Caching.Abstractions/CacheOptions.cs index e397a160..d609b604 100644 --- a/src/UiPath.Caching.Abstractions/CacheOptions.cs +++ b/src/UiPath.Caching.Abstractions/CacheOptions.cs @@ -14,6 +14,10 @@ public class CacheOptions public bool BroadcastEnabled { get; set; } = true; + [SuppressMessage("Major Code Smell", + "S1133:Deprecated code should be removed", + Justification = "Still read, so removing it would relocate every brace-free key.")] + [Obsolete("Changes which keys co-locate, nothing more. Flipping it relocates every brace-free entry.")] public bool ShardKeyEnabled { get; set; } public bool AuditEnabled { get; set; } = true; diff --git a/src/UiPath.Caching/Redis/DefaultRedisKeyStrategyFactory.cs b/src/UiPath.Caching/Redis/DefaultRedisKeyStrategyFactory.cs index aac7f33c..ae02656b 100644 --- a/src/UiPath.Caching/Redis/DefaultRedisKeyStrategyFactory.cs +++ b/src/UiPath.Caching/Redis/DefaultRedisKeyStrategyFactory.cs @@ -27,6 +27,8 @@ public IRedisKeyStrategy Create(CacheOptions options, string differentiator) var keyspace = Guard.NotNullOrWhiteSpace(differentiator, nameof(differentiator)); var separator = Guard.NotWhiteSpace(options.Separator, nameof(options.Separator)); var prefix = string.Join(separator, Guard.NotNullOrWhiteSpace(options.AppShortName, nameof(options.AppShortName)), keyspace); +#pragma warning disable CS0618 // Still honored; see CacheOptions.ShardKeyEnabled. return options.ShardKeyEnabled ? new ShardPrefixRedisKeyStrategy(prefix, separator) : new PrefixRedisKeyStrategy(prefix, separator); +#pragma warning restore CS0618 } } diff --git a/tests/UiPath.Caching.Tests/Broadcast/RedisStreamTopicMonitorTests.cs b/tests/UiPath.Caching.Tests/Broadcast/RedisStreamTopicMonitorTests.cs index 8ee3f422..9e2600ee 100644 --- a/tests/UiPath.Caching.Tests/Broadcast/RedisStreamTopicMonitorTests.cs +++ b/tests/UiPath.Caching.Tests/Broadcast/RedisStreamTopicMonitorTests.cs @@ -85,7 +85,7 @@ public async Task StreamsWithNoConsumerGroups_and_old_message_are_deleted() public void Quarantine_key_is_rendered_as_one_whole_key(bool shardKeyEnabled, string stream, string expected) { // Composing a rendered prefix instead put every quarantine hash under one tag, and so on one slot. - _cacheOptions.ShardKeyEnabled = shardKeyEnabled; + ShardKeyEnabled(shardKeyEnabled); Sut.Initialize(); @@ -136,7 +136,7 @@ public void Quarantine_key_survives_a_separator_that_is_itself_a_brace() // Separator only has to be non-whitespace, so it can be '{' -- which puts braces in the marker, not // just in the stream key, and would reach EnsureTag from there. _cacheOptions.Separator = '{'; - _cacheOptions.ShardKeyEnabled = true; + ShardKeyEnabled(true); // Initialize composes the lock key the same way, so it has to survive the separator too. var initialize = () => Sut.Initialize(); @@ -152,7 +152,7 @@ public async Task A_stream_key_whose_braces_form_no_tag_does_not_abort_the_pass( { // Under ShardKeyEnabled the composed name goes through EnsureTag, which refuses braces that form no // tag; escaping them keeps every stream maintainable instead of losing the whole pass to one of them. - _cacheOptions.ShardKeyEnabled = true; + ShardKeyEnabled(true); _streams = ["tst:st:{}bad", "stream1"]; _database.KeyExistsAsync(Arg.Any(), Arg.Any()).Returns(true); @@ -393,6 +393,10 @@ public ValueTask InitializeAsync() return ValueTask.CompletedTask; } +#pragma warning disable CS0618 // Deprecated but still honored, and the key shape it selects is what these tests cover. + private void ShardKeyEnabled(bool enabled) => _cacheOptions.ShardKeyEnabled = enabled; +#pragma warning restore CS0618 + private StreamInfo GenerateStreamInfo(int? groupsCount = null) { // StreamInfo's internal ctor keeps its leading params (length, radixTreeKeys, radixTreeNodes,