From cb6fccba8df7dca4550c0da86d9fc093cc51f58d Mon Sep 17 00:00:00 2001 From: Cosmin Staicu Date: Wed, 23 Sep 2026 08:43:15 +0300 Subject: [PATCH] feat(cache): open a health window on an announced disruption, and make the read timeout bind A push notification was recorded but never moved health state, so on Redis Enterprise and Redis Cloud a planned handoff still read as a fault. And nothing downstream could act on it: Polly's timeout is cooperative and the Redis client takes no token, so RequestTimeout bounded nothing. An announced disruption now opens a window that reports through InProgress, tracked per operation family and sized by the bounds of the connection that delivered the notice, configurators included. A lapsed operation stays while a live one in its family remains, so a late completion cannot release the running one. Window state is published as one timestamp, so the pipeline's per-operation check takes no lock. MaintenanceRelaxedTimeout and MaintenanceRelaxedWindowMax become settings, and FailFastBacklogPolicy defaults to true; the Azure probe tolerates fail-fast rejections for the hanging time before forcing a reconnect. The read pipeline races its callback against the pipeline token and returns at the timeout; write and custom pipelines never do, since a write can carry memory the caller reclaims and a custom pipeline can guard SPOP. It is not configurable. Each read transaction is built, run and awaited inside the attempt, so a retry never re-runs a drained transaction, and the reads keep the bool pipeline and its breaker. A timeout is not retried, a caller's own cancellation does not count toward the breaker, and DisruptionRequestTimeout widens the cap only while an announced window is open. Every task the library lets go of now has its failure observed: queued transaction commands, background loops, fire-and-forget work and probes abandoned at their timeout. Signed-off-by: Cosmin Staicu --- CHANGELOG.md | 76 +- docs/how-to/resilience.md | 42 +- docs/recipes/redis-health-check.md | 2 +- docs/reference/settings.md | 9 +- .../appsettings.all.json | 15 +- .../Policies/IDisruptionState.cs | 10 + .../Policies/IResiliencePipeline.cs | 3 +- .../PublicAPI.Unshipped.txt | 3 + .../CachingBuilderExtensions.cs | 2 +- .../PublicAPI.Unshipped.txt | 3 + .../ResiliencePipelineFactory.cs | 41 +- .../ResiliencePipelineWrapper.cs | 34 +- .../ResiliencePoliciesOptions.cs | 3 + src/UiPath.Caching.Queue/RedisSetCache.cs | 13 +- src/UiPath.Caching.Queue/TaskObservation.cs | 12 + .../Broadcast/EventDispatcher.cs | 1 + .../Redis/RedisStreamHealthMaintainer.cs | 5 +- .../Redis/RedisStreamSubjectWriter.cs | 1 + src/UiPath.Caching/CacheMemoryMonitor.cs | 1 + .../Config/RedisCollectionExtensions.cs | 1 + src/UiPath.Caching/PublicAPI.Unshipped.txt | 7 + .../IRedisConfigurationOptionsProvider.cs | 3 + src/UiPath.Caching/Redis/RedisCache.cs | 88 +-- .../RedisConfigurationOptionsProvider.cs | 66 +- .../Redis/RedisConnectionConfigurators.cs | 17 +- .../Redis/RedisConnectionOptions.cs | 8 +- .../Redis/RedisConnectionWarmup.cs | 2 +- src/UiPath.Caching/Redis/RedisConnector.cs | 6 +- src/UiPath.Caching/Redis/RedisHashCache.cs | 67 +- .../Redis/RedisPlannedMaintenance.cs | 433 +++++++++- src/UiPath.Caching/RehydrationCoordinator.cs | 2 +- src/UiPath.Caching/TaskObservation.cs | 12 + .../Fakes/RecordingResiliencePipeline.cs | 30 + .../Fakes/RetryOnceResiliencePipeline.cs | 20 + .../Redis/RedisCacheTests.cs | 95 +++ .../Redis/RedisConnectionConfiguratorTests.cs | 105 ++- .../Redis/RedisConnectorTests.cs | 181 +++++ .../Redis/RedisHashCacheTests.cs | 22 + .../RedisPlannedMaintenanceRoutingTests.cs | 737 +++++++++++++++++- .../Redis/RedisSetCacheTests.cs | 51 ++ .../ResiliencePipelineWrapperTests.cs | 235 ++++++ 41 files changed, 2264 insertions(+), 200 deletions(-) create mode 100644 src/UiPath.Caching.Abstractions/Policies/IDisruptionState.cs create mode 100644 src/UiPath.Caching.Queue/TaskObservation.cs create mode 100644 src/UiPath.Caching/TaskObservation.cs create mode 100644 tests/UiPath.Caching.Tests/Fakes/RecordingResiliencePipeline.cs create mode 100644 tests/UiPath.Caching.Tests/Fakes/RetryOnceResiliencePipeline.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index d3875334..a67c2f3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Added +- **`ResiliencePoliciesOptions.DisruptionRequestTimeout`.** Replaces `RequestTimeout` while an announced + window is open, so a cap tight enough to fall through to another tier does not cut short a window the + client is deliberately relaxing its own timeouts for. Resolved per operation through Polly's timeout generator + rather than read at build time, so a window opening after startup is honoured. Left unset, the tier's own + suggestion applies — `IDisruptionState.SuggestedTimeout`, which the Redis tier offers only for an announced + push window, since the Azure probe route opens no native relaxation. The Azure route does not widen the cap + either: its probe run lasts ten minutes whatever the maintenance did. Neither can shorten `RequestTimeout`, and + with no `IDisruptionState` registered one timeout applies throughout. + +- `IDisruptionState`, the two-member seam the pipeline asks whether the tier behind it is disrupted and what it is + relaxing to. Without a registration, behaviour is unchanged. + - **Maintenance notifications from either source.** `RedisPlannedMaintenance` recognised only `AzureMaintenanceEvent`, the pub/sub notifications Azure Cache for Redis publishes, and returned on anything else — so the RESP3 push notifications Redis Enterprise and Redis Cloud send as `PushMaintenanceEvent` were discarded — @@ -20,10 +32,17 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) whichever connection delivered it, the one carrying commands reaching this through the new `IRedisConnector.ServerMaintenance`, and it can be delivered more than once -- Azure's is a broadcast every connection receives, and a push frame is replayed to a connection that reconnects. A copy matching one recorded in - the last 30 seconds is therefore dropped, on the notification's own identity rather than on which connection ought - to have had it. They are recorded rather than acted on — the client relaxes timeouts and hands the connection off itself, and probing would force a reconnect - against that — so `InProgress` is still driven by the Azure route alone. - + the retention window is therefore dropped, on the notification's own identity rather than on which connection + ought to have had it. The retention is the maximum window plus its post-event tail, floored at 30 seconds -- + see the fix below for why a fixed interval was not enough. A push notification opens a maintenance window rather than probing — the client hands the + connection off itself, and probing would force a reconnect against it — for as long as the server announced, + clamped to the range the client relaxes its own timeouts over (`MaintenanceRelaxedTimeout` to + `MaintenanceRelaxedWindowMax`) and held through `MaintenancePostEventRelaxedDuration` after a completion, so the + two agree by construction; and tracked per operation family, so one completion cannot close another's window. The + bounds are read from the connection that delivered the notification, configurators included, so a window is sized + right from the first notice rather than by defaults until this service's own connection is up. The + two routes report one pair of `Redis.MaintenanceStarted`/`Ended` events between them rather than one pair each, + since they can overlap. - `IRedisConnector.ServerMaintenance`, the maintenance the server announced on the connection carrying commands. Defaulted to never raising, so an existing implementer is unaffected. It exists because the connector is what rebuilds that connection, so one subscription here survives a `ForceReconnect` where subscribing to the @@ -33,15 +52,34 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Changed +- **The read pipeline now returns at `RequestTimeout`.** Polly's timeout is cooperative: it bounds a callback that + observes the token it arms, and the Redis client takes no cancellation token, so on those paths `RequestTimeout` + bounded nothing and the retry, breaker and fallback downstream never saw a failure to act on. The read pipeline + now races the callback against that token, which carries the request timeout and the caller's own token alike, + so control returns to the caller when either fires. This is fixed rather than configurable, and only the read + pipeline does it: a write can carry memory the caller reclaims on return, and a custom pipeline can guard a + destructive read such as `SPOP`. The abandoned call still runs underneath and its later failure is observed. + A timeout is no longer retried, since the abandoned command is still in flight, and a caller's own cancellation + no longer counts toward the circuit breaker. **This changes caller-visible timing on reads:** an operation that + previously ran past `RequestTimeout` now returns at it. + +- **`RedisConnectionOptions.FailFastBacklogPolicy` now defaults to `true`.** Queuing while disconnected makes a + command wait out the connection's timeout with nothing to wait for. Set `false` to restore + `BacklogPolicy.Default`. **This changes behaviour for disconnected commands**: they now fail fast rather than + queue. `null` no longer means the library default: it fails fast like `true`, and so does a JSON `null`, as the + shipped `appsettings.all.json` had it, whether the binder leaves the new default or writes `null`. Only `false` + restores queuing. The Azure maintenance probe allows for this: a probe rejected while the client is reconnecting + on its own no longer forces a reconnect, unless the disconnect outlasts the ten-second hanging time. + + - **A maintenance handoff is no longer reported as a connection failure.** When the client moves off an endpoint the server said is going away, it raises `ConnectionFailed` with `ConnectionFailureType.MaintenanceHandoff`. That was tracked as `Redis.ConnectionFailed` — by `RedisConnector` and again by `ConnectionStateMonitor` — which would alert on exactly the event advance notice exists to make uneventful. Both now track it as `Redis.MaintenanceHandoff`. The event is still raised to subscribers — the connection did drop — and only the telemetry name distinguishes them. -- `RedisHealthCheck` reports "Redis maintenance in progress" rather than naming Azure Cache for Redis. The wording - is provider-neutral in readiness for the push route; the state behind it is not yet, since only the Azure route - opens it here. +- `RedisHealthCheck` reports "Redis maintenance in progress" rather than naming Azure Cache for Redis, now that + either route can open the window. - Bumped `StackExchange.Redis` from 3.2.1 to 3.3.0. No public API change here, and nothing in this repository calls an API 3.2.15 or 3.3.0 altered. Three things in the range are worth knowing: - `SwitchPrimary` now retires the servers its rebuild drops (upstream #3225). They previously stayed in the server @@ -58,6 +96,30 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Fixed +- **No task the library lets go of can surface as an unobserved exception.** Commands queued on a transaction, + background loops, fire-and-forget work and probes abandoned at their timeout were discarded without anything + observing a later failure, which `TaskScheduler.UnobservedTaskException` then reported. Each now observes its own. + +- **A refused `Redis.Maintenance` record no longer releases the notification's deduplication claim.** + Recording a notification also opens and closes health windows, so releasing the claim when the record + failed let a replayed frame apply the same state change twice — a second operation no single completion + closes, or a close while maintenance is still running. The record is guarded instead: a refused one is + reported and dropped, and the claim stands. Losing an event is the lesser harm. +- **`MaintenanceRelaxedWindowMax` caps a larger `MaintenanceRelaxedTimeout`.** The floor was applied after + the maximum, so an announcement below the floor returned the floor even when it exceeded the documented + cap, and health suppression outlived the window the client actually relaxes over. +- **A replayed push frame could open a second operation or close someone else's.** The deduplication claim + was retained for a fixed 30 seconds while an announced window can run far longer, so a frame replayed to a + reconnecting connection after the claim lapsed was taken as new: a starter appended an operation the one + real completion could not close, and a completion removed another operation still running. The retention + now covers `MaintenanceRelaxedWindowMax` plus `MaintenancePostEventRelaxedDuration`, the full span over which + a window can still be holding the state, whenever that exceeds the 30-second floor. +- **An explicit `maintRelaxedTimeout` in the connection string is no longer derived over.** With the typed + option unset, the derived value replaced whatever the string supplied — while `maintRelaxedWindowMax` in + the same string was preserved. A value from either input now counts as configured, detected by the key's + presence rather than its value — an explicit setting that happens to equal the client's default is still + a setting. + - **A throwing subscriber cost the remaining ones their notification.** `RedisConnector` and `ConnectionStateMonitor` raised their connection events with a plain multicast invoke, which stops at the first handler that throws; `ForceReconnect` caught the exception, but around the whole invocation list rather than around diff --git a/docs/how-to/resilience.md b/docs/how-to/resilience.md index cf28ad03..cf11251d 100644 --- a/docs/how-to/resilience.md +++ b/docs/how-to/resilience.md @@ -311,8 +311,9 @@ See [reference/settings.md](../reference/settings.md) for the full reference. ### Retries and non-idempotent operations -`RetryCount` defaults to `1` and the retry strategy handles **every** exception, including the -per-attempt `RequestTimeout` (`TimeoutRejectedException`). For idempotent commands this is safe — +`RetryCount` defaults to `1` and the retry strategy handles every exception except the per-attempt +`RequestTimeout` (`TimeoutRejectedException`), whose command is still in flight, and a cancellation the +caller asked for. For idempotent commands this is safe — replaying `SADD`/`SREM`/`SET`/`DEL`/`EXPIRE` converges to the same end state (only the returned count may differ). It is **not** safe for destructive-read commands like `SPOP` (and the planned `LPOP`/`RPOP`): if the server executed the pop but the response was lost to a timeout or a @@ -321,7 +322,8 @@ silent data loss. Such operations therefore do **not** go through the retrying `Write` pipeline. The set cache resolves a configurable pipeline via `IResiliencePipelineProvider.Get(RedisSetCacheOptions.ResilienceKeyName)`. -Register your own pipeline for that name and point the set cache at it: +Register your own pipeline for that name and point the set cache at it. The built-in `read` pipeline is refused, since it +abandons a call at its timeout and an abandoned `SPOP` loses what it popped: ```csharp builder @@ -423,7 +425,7 @@ old one (`Redis.ForcedReconnect` event, `OnReconnected` raised). |---|---|---| | Hang detection | More than 100 commands awaiting a reply on the primary with no read or write for `LastWrite/ReadIntervalThresholdMilliseconds` | `EnableHangDetection`, `HangDetectionDueTime`, `HangDetectionPeriod` | | Planned maintenance | `NodeMaintenanceStarting` on the `AzureRedisEvents` channel; probes with a write every second for 10 minutes and reconnects on failure | `PlannedMaintenanceEnabled` | -| Announced maintenance | RESP3 push notifications on the command connection, from Redis Enterprise and Redis Cloud (Azure Managed Redis once its rollout lands); recorded, while the client relaxes timeouts and hands the connection off | `PlannedMaintenanceEnabled`, `MaintenanceNotifications` | +| Announced maintenance | RESP3 push notifications on the command connection, from Redis Enterprise and Redis Cloud (Azure Managed Redis once its rollout lands); opens a health window while the client relaxes timeouts and hands the connection off | `PlannedMaintenanceEnabled`, `MaintenanceNotifications` | | Stale endpoint detection | A topology-discovered node has been disconnected for `StaleEndpointThreshold` and is no longer in the cluster topology the client refreshes | `EnableStaleEndpointDetection`, `StaleEndpointThreshold`, `StaleEndpointScanInterval` | **Stale endpoints** are the clustered-cache failure mode. StackExchange.Redis discovers the @@ -469,9 +471,9 @@ Redis Enterprise and Redis Cloud instead send RESP3 push notifications on the co your commands, and none of them publish `AzureRedisEvents`. The client acts on these itself — relaxing timeouts, re-reading topology, moving off a departing endpoint — so this library records them and leaves the recovery alone: probing force-reconnects on a failed write, which would fight -the handoff. Azure Managed Redis (`*.redis.azure.net`) is recognised as a provider but nothing -turns the request on for it, so `MaintenanceNotifications` below is what asks. Reporting the -disruption through `InProgress` is a separate change. +the handoff. It opens a maintenance window instead, so health reporting does not call the handoff a +fault while it is in progress. Azure Managed Redis (`*.redis.azure.net`) is recognised as a +provider but nothing turns the request on for it, so `MaintenanceNotifications` below is what asks. A notification can arrive more than once: Azure's is a broadcast every connection receives, and a push frame is replayed to a connection that reconnects, which the client collapses only within the @@ -488,6 +490,32 @@ connection carries no commands. And only a `MOVING` is tied to a connection gene carrying commands or the one about to, since a rebuild subscribes the replacement before publishing it and the server never replays a `MOVING`. +The command route is also the only one that opens a window, for the same reason: it is the +connection whose disruption the cache would feel. + +A window lasts as long as the server announced, clamped to the range the client relaxes its own +timeouts over — `maintRelaxedTimeout` to `maintRelaxedWindowMax` — and a completion hands over to +`maintPostEventRelaxed` rather than closing at once, because the client keeps treating failures as +maintenance for that tail. Health reporting therefore stays quiet for exactly as long as the client +stays patient, and follows those settings if you change them. +The clamp matters because durations of a couple of seconds have been observed, which is not long +enough to cover a reconnect, and a client that trusted one would stop being patient exactly when +it mattered. Each +operation is tracked by family, so a `MIGRATED` closes the migration it started and not a +failover still in flight. `MOVING` has no completion at all and can only lapse. + +The push route is opt-in. Set `MaintenanceNotifications` to `Auto` to ask for it and connect +normally if the server does not offer it, or `Required` to refuse a connection that will not +deliver them — useful for proving the feature is live in staging. + +`Auto` is the safe default everywhere: on a server that does not emit these — Redis Open Source, +Valkey, and Azure Managed Redis until its rollout lands — the opt-in is simply refused and the +connection carries on. `Required` on such a server **rejects the connection**, so keep it to +environments where you know the server emits them. It needs RESP3, which is +negotiated by default. Forcing RESP2, or a `DefaultVersion` below 6.0, takes RESP3 away: under +`Auto` that disables the notifications silently, while `Required` rejects the connection +outright, which is the other reason to reach for it in staging. + A handoff surfaces as a `ConnectionFailed` event with `ConnectionFailureType.MaintenanceHandoff`. It is tracked as `Redis.MaintenanceHandoff` rather than `Redis.ConnectionFailed`, so planned maintenance does not raise a failure alert, but it is diff --git a/docs/recipes/redis-health-check.md b/docs/recipes/redis-health-check.md index 9db67e13..04d42825 100644 --- a/docs/recipes/redis-health-check.md +++ b/docs/recipes/redis-health-check.md @@ -36,7 +36,7 @@ A 3-second timeout is the conventional value — longer than a typical ping (sub The healthy result carries the multiplexer's `IsConnected`, `IsConnecting`, `OperationCount`, `Status` and `DisconnectedEndPoints` (a `;`-joined `host:port` list) in its data, so a probe that is green can still show a node the multiplexer cannot reach. A discovered node that stays in that list after a cluster patch is what [stale endpoint detection](../how-to/resilience.md#redis-connection-self-healing) removes by rebuilding the connection; the health check does not need to fail for that to happen. -`IRedisPlannedMaintenance.InProgress` becomes `true` on one route only. Azure Cache for Redis Basic/Standard/Premium publish the `AzureRedisEvents` channel, and a node going away there starts the probe loop that holds the state open. Redis Enterprise and Redis Cloud instead send RESP3 push notifications on the connection carrying commands; those are recorded but do not yet move this state, so on that route the check behaves as if no maintenance tracker were registered. Azure Managed Redis is recognised as a push provider but its servers do not emit these yet either. +`IRedisPlannedMaintenance.InProgress` becomes `true` two ways. Azure Cache for Redis Basic/Standard/Premium publish the `AzureRedisEvents` channel, and a node going away there starts the probe loop that holds the state open. Redis Enterprise and Redis Cloud instead send RESP3 push notifications on the connection carrying commands, and one of those opens a window for as long as the client relaxes its own timeouts; that route is opt-in through `MaintenanceNotifications` (see [resilience.md](../how-to/resilience.md#redis-connection-self-healing)). Azure Managed Redis is recognised as a push provider but its servers do not emit these yet, so until that rollout lands the check there behaves as if no maintenance tracker were registered. ## When not to use diff --git a/docs/reference/settings.md b/docs/reference/settings.md index eb9a19ee..f6469089 100644 --- a/docs/reference/settings.md +++ b/docs/reference/settings.md @@ -56,6 +56,8 @@ Every binding-visible property on every shipped options class, with shipped defa | `ProfilerFeatureFlagKey` | `string` | `"RedisProfiler.Enabled"` | App-wide | Feature-flag key consulted before enabling the StackExchange.Redis command profiler. | | `PlannedMaintenanceEnabled` | `bool` | `true` | App-wide | Tolerate planned-maintenance disconnects gracefully instead of faulting. | | `MaintenanceNotifications` | `RedisMaintenanceNotifications?` | `null` | App-wide | Ask the server for advance notice of maintenance. `null` leaves the client's own default, so provider enlistment applies as it lands upstream. `Auto` asks and connects normally if the server does not offer them. `Required` refuses a connection that will not deliver them — useful to prove the feature is live. Delivered as RESP3 push notifications by Redis Enterprise and Redis Cloud; Azure Managed Redis is recognised as that kind of provider but its servers do not emit them yet, so the opt-in is preparatory there. Azure Cache for Redis uses the `AzureRedisEvents` channel instead and needs no opt-in. On a server that does not emit them — Redis OSS, Valkey, AMR for now — `Auto` is refused and the connection carries on, while `Required` rejects it. One exception, and it is the client's: inside a multi-group (geo-redundant) connection the feature is not activated at all, so `Required` warns and connects rather than rejecting — failing there would leave a group with no way to opt in, and the restriction is expected to be lifted upstream. Requires RESP3, which is negotiated by default. | +| `MaintenanceRelaxedTimeout` | `TimeSpan?` | `null` | App-wide | How long the client keeps command timeouts relaxed while an announced maintenance window is open. `null` derives twice the effective async timeout; with no connection string to derive from, the client's own default (10s) is left alone. Configured or derived, the value is rounded up into the whole seconds the option is expressed in and clamped to the 1-600s range the client parses, so a sub-second value becomes 1s rather than the `0` the client would otherwise read back. The two values the client derives from it move with it unless they are set themselves: `MaintenanceRelaxedWindowMax` (3x) and how long timeouts stay relaxed after a disruption completes (2x). | +| `MaintenanceRelaxedWindowMax` | `TimeSpan?` | `null` | App-wide | Caps a relaxed window however long the server announced the disruption would take -- the backstop for a closing notification that never arrives. `null` leaves the client deriving it as three times the effective relaxed timeout, which keeps the two coherent. Set, it is rounded up and clamped like the relaxed timeout above. Nothing checks it against the timeout it caps: set below that, every announced window collapses to this value. | | `PlannedMaintenanceConnectionRetryCount` | `int` | `5` | App-wide | Attempts to establish the planned-maintenance subscription before backing off to quiet retries; failures are logged as warnings, never faulting startup. | | `PlannedMaintenanceConnectionRetryDelay` | `TimeSpan` | `00:00:05` | App-wide | Delay between planned-maintenance subscription attempts (negative/zero is clamped to 1s). | | `LogConnectionFailedEvents` | `bool` | `true` | App-wide | Log `ConnectionFailed` events from the multiplexer. | @@ -69,7 +71,7 @@ Every binding-visible property on every shipped options class, with shipped defa | `EnableStaleEndpointDetection` | `bool` | `true` | App-wide | Rebuild the connection when a cluster node the multiplexer discovered has left the cluster but is still being retried (see [Redis connection self-healing](../how-to/resilience.md#redis-connection-self-healing)). Works under the default `allowAdmin=false`; emits one `Redis.StaleEndpointScanDisabled` event and stops when no refreshed node reports a cluster configuration. | | `StaleEndpointThreshold` | `TimeSpan` | `00:05:00` | App-wide | How long a discovered endpoint must stay disconnected before its cluster membership is checked; non-positive falls back to the default. | | `StaleEndpointScanInterval` | `TimeSpan` | `00:00:30` | App-wide | Period between stale-endpoint scans; non-positive falls back to the default. | -| `FailFastBacklogPolicy` | `bool?` | `null` | App-wide | `null` = library default; `true` = fail immediately when the command backlog is full. | +| `FailFastBacklogPolicy` | `bool?` | `true` | App-wide | Fail a command immediately when the connection is unavailable, rather than queuing it in the backlog until the connection's own timeout elapses. `null`, including a JSON `null`, fails fast like `true`; only `false` restores the client's queuing default. | | `ProfilerEnabled` | `bool` | `false` | App-wide | Enable StackExchange.Redis command profiler. | | `ProfilerHasDefaultSession` | `bool` | `true` | App-wide | Start a default profiling session automatically at startup. | | `ProfilerFlushInterval` | `TimeSpan` | `00:00:01` | App-wide | How often profiling data is flushed to the sink. | @@ -249,7 +251,7 @@ The queue package's `AddQueueMemory` / `AddQueueRedis` / `AddQueueInMemoryRedis` | Property | Type | Default | Scope | Notes | |---|---|---|---|---| | `Enabled` | `bool` | `true` | Per-provider | Enable/disable the Redis set cache. | -| `ResilienceKeyName` | `string?` | `null` | Per-provider | Name of the resilience pipeline applied to destructive reads (`SPOP`), resolved via `IResiliencePipelineProvider`; `null` or empty runs them with no pipeline. | +| `ResilienceKeyName` | `string?` | `null` | Per-provider | Name of the resilience pipeline applied to destructive reads (`SPOP`), resolved via `IResiliencePipelineProvider`; `null` or empty runs them with no pipeline. The built-in `read` pipeline is refused: it abandons a call at its timeout, and an abandoned `SPOP` loses what it popped. | Lifetimes and the connection come from `RedisCacheOptions` (`DefaultExpiration`, `ConnectionMonitorEnabled`, …) bound from the same section. @@ -384,7 +386,8 @@ scenarios, not for long-lived entries. | `Enabled` | `bool` | `true` | App-wide | Enable Polly circuit-breaker and retry policies for Redis operations. | | `DurationOfBreak` | `TimeSpan` | `00:01:00` | App-wide | How long the circuit stays open after tripping. | | `ExceptionsAllowedBeforeBreaking` | `int` | `500` | App-wide | Number of failures within the sampling window before the circuit opens. | -| `RequestTimeout` | `TimeSpan?` | `00:00:01` | App-wide | Per-operation timeout enforced by the Polly pipeline. | +| `RequestTimeout` | `TimeSpan?` | `00:00:01` | App-wide | Per-operation timeout enforced by the Polly pipeline. The strategy is cooperative: it bounds a callback that observes the token it arms, and on this library's Redis paths the client takes no cancellation token, so the read pipeline races the callback against it and returns at the timeout, leaving the call to finish underneath. Write and custom pipelines do not, since a write can carry memory the caller reclaims on return and a custom pipeline can guard a destructive read such as `SPOP`; there it bounds nothing. | +| `DisruptionRequestTimeout` | `TimeSpan?` | `null` | Per-pipeline | Replaces `RequestTimeout` while an announced window is open, so a window the client is deliberately relaxing timeouts for is not cut short by the app's own cap. Resolved per operation, so it follows a window that opens after startup. Left `null`, the tier's own suggestion applies instead: the Redis tier offers its effective `MaintenanceRelaxedTimeout`, which is what it is relaxing its command timeouts to, so the two stay coherent without being configured twice. The Azure probe route does not widen it, since that run lasts ten minutes whatever the maintenance did. Neither the configured value nor the suggestion can shorten `RequestTimeout`. With no `IDisruptionState` registered, one timeout applies throughout. | | `RetryCount` | `int?` | `1` | App-wide | Number of immediate retries before propagating a failure. | | `TelemetryEnabled` | `bool` | `true` | App-wide | Emit circuit-breaker state-change events via the telemetry provider. | | `RethrowCircuitBreakerExceptions` | `bool` | `false` | App-wide | `true` = rethrow `BrokenCircuitException` to the caller instead of swallowing it. | diff --git a/samples/UiPath.Caching.Sample/appsettings.all.json b/samples/UiPath.Caching.Sample/appsettings.all.json index 22c44e3b..1c280c3e 100644 --- a/samples/UiPath.Caching.Sample/appsettings.all.json +++ b/samples/UiPath.Caching.Sample/appsettings.all.json @@ -61,6 +61,14 @@ // A multi-group (geo-redundant) connection is the exception: the client does not // activate the feature there, so "Required" warns and connects rather than rejecting. "MaintenanceNotifications": null, + // MaintenanceRelaxedTimeout: how patient the connection stays while a window is open. + // null = twice the effective async timeout, so a connection told to give up quickly + // stays proportionally patient. A value here, or maintRelaxedTimeout in the + // connection string, is taken as configured and not derived over. + "MaintenanceRelaxedTimeout": null, + // MaintenanceRelaxedWindowMax: the longest a window may run, whatever the server announced. + // null = left to the client, which keeps it a multiple of the relaxed timeout above. + "MaintenanceRelaxedWindowMax": null, // LogConnectionFailedEvents: log ConnectionFailed events from the multiplexer "LogConnectionFailedEvents": true, // LogConnectionRestoredEvents: log ConnectionRestored events from the multiplexer @@ -83,7 +91,8 @@ "StaleEndpointThreshold": "00:05:00", // StaleEndpointScanInterval: period between stale-endpoint scans "StaleEndpointScanInterval": "00:00:30", - // FailFastBacklogPolicy: null = library default; true = fail immediately when backlog is full + // FailFastBacklogPolicy: null and true both fail fast; false restores BacklogPolicy.Default, + // which queues while disconnected and makes a command wait out the connection timeout. "FailFastBacklogPolicy": null, // ProfilerEnabled: enable StackExchange.Redis command profiler "ProfilerEnabled": false, @@ -319,6 +328,10 @@ "ExceptionsAllowedBeforeBreaking": 500, // RequestTimeout: per-operation timeout enforced by the Polly pipeline "RequestTimeout": "0:00:01", + // DisruptionRequestTimeout: replaces RequestTimeout while an announced push window is open. + // null = the tier's own suggestion. The Azure probe route does not widen it. Neither can + // shorten RequestTimeout. + "DisruptionRequestTimeout": null, // RetryCount: number of immediate retries before propagating failure "RetryCount": 1, // TelemetryEnabled: emit circuit-breaker state-change events via telemetry diff --git a/src/UiPath.Caching.Abstractions/Policies/IDisruptionState.cs b/src/UiPath.Caching.Abstractions/Policies/IDisruptionState.cs new file mode 100644 index 00000000..838e8903 --- /dev/null +++ b/src/UiPath.Caching.Abstractions/Policies/IDisruptionState.cs @@ -0,0 +1,10 @@ +namespace UiPath.Caching.Policies; + +/// Whether the tier behind a pipeline is disrupted. +public interface IDisruptionState +{ + bool InProgress { get; } + + /// What the tier relaxes its timeouts to while disrupted, if anything. + TimeSpan? SuggestedTimeout { get; } +} diff --git a/src/UiPath.Caching.Abstractions/Policies/IResiliencePipeline.cs b/src/UiPath.Caching.Abstractions/Policies/IResiliencePipeline.cs index 573019a0..c91f0db9 100644 --- a/src/UiPath.Caching.Abstractions/Policies/IResiliencePipeline.cs +++ b/src/UiPath.Caching.Abstractions/Policies/IResiliencePipeline.cs @@ -1,6 +1,7 @@ namespace UiPath.Caching.Policies; -/// must await callback to completion even on timeout: the Redis tier hands the connection borrowed memory that the caller reclaims once the pipeline returns. +/// The read pipeline returns when its token is cancelled, leaving callback running; others wait. +/// A callback holding memory the caller reclaims on return must not use the read pipeline. public interface IResiliencePipeline { ValueTask ExecuteAsync(Func> callback, TResult defaultValue, CancellationToken cancellationToken = default); diff --git a/src/UiPath.Caching.Abstractions/PublicAPI.Unshipped.txt b/src/UiPath.Caching.Abstractions/PublicAPI.Unshipped.txt index 7dc5c581..505da9bf 100644 --- a/src/UiPath.Caching.Abstractions/PublicAPI.Unshipped.txt +++ b/src/UiPath.Caching.Abstractions/PublicAPI.Unshipped.txt @@ -1 +1,4 @@ #nullable enable +UiPath.Caching.Policies.IDisruptionState +UiPath.Caching.Policies.IDisruptionState.InProgress.get -> bool +UiPath.Caching.Policies.IDisruptionState.SuggestedTimeout.get -> System.TimeSpan? diff --git a/src/UiPath.Caching.Polly/CachingBuilderExtensions.cs b/src/UiPath.Caching.Polly/CachingBuilderExtensions.cs index d395d769..2e3dcd03 100644 --- a/src/UiPath.Caching.Polly/CachingBuilderExtensions.cs +++ b/src/UiPath.Caching.Polly/CachingBuilderExtensions.cs @@ -39,7 +39,7 @@ public static ICachingBuilder AddResilienceStrategies(this ICachingBuilder build ? sp.GetRequiredService>().Value : null; - return new ResiliencePipelineFactory(loggerFactory, telemetryOptions, resilienceOptions); + return new ResiliencePipelineFactory(loggerFactory, telemetryOptions, resilienceOptions, sp.GetService()); }); // Predefined pipelines, seeded with the same base configuration. diff --git a/src/UiPath.Caching.Polly/PublicAPI.Unshipped.txt b/src/UiPath.Caching.Polly/PublicAPI.Unshipped.txt index 7dc5c581..e4731d4f 100644 --- a/src/UiPath.Caching.Polly/PublicAPI.Unshipped.txt +++ b/src/UiPath.Caching.Polly/PublicAPI.Unshipped.txt @@ -1 +1,4 @@ #nullable enable +UiPath.Caching.Polly.ResiliencePipelineFactory.ResiliencePipelineFactory(Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory, Polly.Telemetry.TelemetryOptions? telemetryOptions, Microsoft.Extensions.Options.IOptionsMonitor! optionsAccessor, UiPath.Caching.Policies.IDisruptionState? disruptionState) -> void +UiPath.Caching.Polly.ResiliencePoliciesOptions.DisruptionRequestTimeout.get -> System.TimeSpan? +UiPath.Caching.Polly.ResiliencePoliciesOptions.DisruptionRequestTimeout.set -> void diff --git a/src/UiPath.Caching.Polly/ResiliencePipelineFactory.cs b/src/UiPath.Caching.Polly/ResiliencePipelineFactory.cs index 8bb2f200..5cff04b3 100644 --- a/src/UiPath.Caching.Polly/ResiliencePipelineFactory.cs +++ b/src/UiPath.Caching.Polly/ResiliencePipelineFactory.cs @@ -2,15 +2,26 @@ using Polly.Fallback; using Polly.Retry; using Polly.Telemetry; +using Polly.Timeout; +using UiPath.Caching.Policies; namespace UiPath.Caching.Polly; public class ResiliencePipelineFactory( ILoggerFactory loggerFactory, TelemetryOptions? telemetryOptions, - IOptionsMonitor optionsAccessor) + IOptionsMonitor optionsAccessor, + IDisruptionState? disruptionState) : IResiliencePipelineFactory { + public ResiliencePipelineFactory( + ILoggerFactory loggerFactory, + TelemetryOptions? telemetryOptions, + IOptionsMonitor optionsAccessor) + : this(loggerFactory, telemetryOptions, optionsAccessor, null) + { + } + protected ResiliencePoliciesOptions ResilienceOptions => optionsAccessor.CurrentValue; protected ILoggerFactory LoggerFactory => loggerFactory; @@ -50,7 +61,7 @@ protected virtual ResiliencePipelineBuilder GetBuilder(ILogger { builder.AddCircuitBreaker(new CircuitBreakerStrategyOptions { - ShouldHandle = new PredicateBuilder().Handle(), + ShouldHandle = args => ValueTask.FromResult(IsFailure(args.Outcome.Exception, args.Context)), BreakDuration = resilienceOptions.DurationOfBreak, MinimumThroughput = resilienceOptions.ExceptionsAllowedBeforeBreaking, OnHalfOpened = args => @@ -75,7 +86,8 @@ protected virtual ResiliencePipelineBuilder GetBuilder(ILogger { builder.AddRetry(new RetryStrategyOptions { - ShouldHandle = new PredicateBuilder().Handle(), + // Not a timeout: the abandoned read is still in flight. + ShouldHandle = args => ValueTask.FromResult(args.Outcome.Exception is not TimeoutRejectedException && IsFailure(args.Outcome.Exception, args.Context)), MaxRetryAttempts = resilienceOptions.RetryCount!.Value, BackoffType = DelayBackoffType.Constant, DelayGenerator = args => ValueTask.FromResult(TimeSpan.FromMilliseconds(args.AttemptNumber * 100)), @@ -89,9 +101,15 @@ protected virtual ResiliencePipelineBuilder GetBuilder(ILogger if (resilienceOptions.RequestTimeout.GetValueOrDefault() > TimeSpan.Zero) { + var requestTimeout = resilienceOptions.RequestTimeout.GetValueOrDefault(); builder.AddTimeout(new TimeoutStrategyOptions { - Timeout = resilienceOptions.RequestTimeout.GetValueOrDefault(), + Timeout = requestTimeout, + + // Per execution: a window can open after the pipeline is built. + TimeoutGenerator = disruptionState is null + ? null + : _ => ValueTask.FromResult(ResolveTimeout(disruptionState, resilienceOptions, requestTimeout)), OnTimeout = args => { logger.LogWarning("Execution timed out after {TotalMilliseconds} ms. Operation key {OperationKey}", args.Timeout.TotalMilliseconds, args.Context.OperationKey); @@ -108,4 +126,19 @@ protected virtual ResiliencePipelineBuilder GetBuilder(ILogger return builder; } + + private static bool IsFailure(Exception? exception, ResilienceContext context) => + exception is not null && !(exception is OperationCanceledException && context.CancellationToken.IsCancellationRequested); + + private static TimeSpan ResolveTimeout(IDisruptionState disruptionState, ResiliencePoliciesOptions resilienceOptions, TimeSpan requestTimeout) + { + // Announced windows only: the Azure probe run outlasts the maintenance. + if (disruptionState.SuggestedTimeout is not { } suggested) + { + return requestTimeout; + } + + var widened = resilienceOptions.DisruptionRequestTimeout ?? suggested; + return widened > requestTimeout ? widened : requestTimeout; + } } diff --git a/src/UiPath.Caching.Polly/ResiliencePipelineWrapper.cs b/src/UiPath.Caching.Polly/ResiliencePipelineWrapper.cs index 2818350f..660ffd60 100644 --- a/src/UiPath.Caching.Polly/ResiliencePipelineWrapper.cs +++ b/src/UiPath.Caching.Polly/ResiliencePipelineWrapper.cs @@ -7,10 +7,42 @@ internal sealed class ResiliencePipelineWrapper(IResiliencePipelineFactory facto { private readonly ConcurrentDictionary<(Type,object?), object> _cachePipeline = new(); + // Reads only: abandoning a write or an SPOP would lose data. + private readonly bool _abandonOnCancellation = string.Equals(scope, ResiliencePipelineNames.Read, StringComparison.Ordinal); + + // Polly's timeout is cooperative, and the Redis client takes no token. public ValueTask ExecuteAsync(Func> callback, TResult defaultValue, CancellationToken cancellationToken = default) { var pipeline = GetPipeline(defaultValue); - return pipeline.ExecuteAsync(callback, cancellationToken); + return _abandonOnCancellation + ? pipeline.ExecuteAsync(token => RaceCancellation(callback, token), cancellationToken) + : pipeline.ExecuteAsync(callback, cancellationToken); + } + + /// Returns when the token fires, leaving the callback running. + private static async ValueTask RaceCancellation(Func> callback, CancellationToken token) + { + var pending = callback(token); + if (pending.IsCompleted || !token.CanBeCanceled) + { + return await pending.ConfigureAwait(false); + } + + var task = pending.AsTask(); + try + { + return await task.WaitAsync(token).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + // WaitAsync stops observing the task. + _ = task.ContinueWith( + static abandoned => _ = abandoned.Exception, + CancellationToken.None, + TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + throw; + } } private ResiliencePipeline GetPipeline(TResult defaultValue) diff --git a/src/UiPath.Caching.Polly/ResiliencePoliciesOptions.cs b/src/UiPath.Caching.Polly/ResiliencePoliciesOptions.cs index 12af7b58..3ff68991 100644 --- a/src/UiPath.Caching.Polly/ResiliencePoliciesOptions.cs +++ b/src/UiPath.Caching.Polly/ResiliencePoliciesOptions.cs @@ -15,4 +15,7 @@ public class ResiliencePoliciesOptions public bool TelemetryEnabled { get; set; } = true; public bool RethrowCircuitBreakerExceptions { get; set; } + + /// Replaces during an announced window; unset uses the tier's suggestion. + public TimeSpan? DisruptionRequestTimeout { get; set; } } diff --git a/src/UiPath.Caching.Queue/RedisSetCache.cs b/src/UiPath.Caching.Queue/RedisSetCache.cs index fa716a07..796d964e 100644 --- a/src/UiPath.Caching.Queue/RedisSetCache.cs +++ b/src/UiPath.Caching.Queue/RedisSetCache.cs @@ -32,6 +32,12 @@ public RedisSetCache( _logger = logger; _read = resiliencePipelineProvider.Get(ResiliencePipelineNames.Read); _write = resiliencePipelineProvider.Get(ResiliencePipelineNames.Write); + if (string.Equals(setCacheOptions.ResilienceKeyName, ResiliencePipelineNames.Read, StringComparison.Ordinal)) + { + throw new InvalidOperationException( + $"{nameof(RedisSetCacheOptions)}.{nameof(RedisSetCacheOptions.ResilienceKeyName)} cannot be '{ResiliencePipelineNames.Read}': that pipeline abandons calls at its timeout, and a destructive read such as SPOP would lose the items it popped. Register a pipeline of your own."); + } + _pop = resiliencePipelineProvider.Get(setCacheOptions.ResilienceKeyName); _cacheOptions = cacheOptions; _redisKeyStrategy = (redisCacheOptions.RedisKeyStrategyFactory ?? new DefaultRedisKeyStrategyFactory()).Create(_cacheOptions, RedisSetKeyspace); @@ -367,10 +373,10 @@ private static void QueueExpirationUpdate(ITransaction transaction, RedisKey red { if (expiration != DateTimeOffset.MaxValue) { - _ = transaction.KeyExpireAsync(redisKey, expiration.UtcDateTime, CommandFlags.DemandMaster | CommandFlags.FireAndForget).ConfigureAwait(false); + transaction.KeyExpireAsync(redisKey, expiration.UtcDateTime, CommandFlags.DemandMaster | CommandFlags.FireAndForget).Forget(); return; } - _ = transaction.KeyPersistAsync(redisKey, CommandFlags.DemandMaster | CommandFlags.FireAndForget).ConfigureAwait(false); + transaction.KeyPersistAsync(redisKey, CommandFlags.DemandMaster | CommandFlags.FireAndForget).Forget(); } private ValueTask AddCoreAsync(CacheKey cacheKey, IEnumerable items, DateTimeOffset expiration, CancellationToken token) @@ -408,6 +414,9 @@ private async ValueTask AddManyInnerAsync(CacheKey cacheKey, RedisValue { var transaction = Database.CreateTransaction(); var addTask = transaction.SetAddAsync(redisKey, values, CommandFlags.DemandMaster); + + // Observed now: an uncommitted transaction leaves it unawaited. + addTask.Forget(); QueueExpirationUpdate(transaction, redisKey, expiration); var committed = await _write.ExecuteAsync(async token => diff --git a/src/UiPath.Caching.Queue/TaskObservation.cs b/src/UiPath.Caching.Queue/TaskObservation.cs new file mode 100644 index 00000000..e9b627a3 --- /dev/null +++ b/src/UiPath.Caching.Queue/TaskObservation.cs @@ -0,0 +1,12 @@ +namespace UiPath.Caching.Redis; + +internal static class TaskObservation +{ + /// Observes the failure of a task nothing awaits. + public static void Forget(this Task task) => + _ = task.ContinueWith( + static failed => _ = failed.Exception, + CancellationToken.None, + TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); +} diff --git a/src/UiPath.Caching/Broadcast/EventDispatcher.cs b/src/UiPath.Caching/Broadcast/EventDispatcher.cs index ec36633b..fb70ef82 100644 --- a/src/UiPath.Caching/Broadcast/EventDispatcher.cs +++ b/src/UiPath.Caching/Broadcast/EventDispatcher.cs @@ -26,6 +26,7 @@ public EventDispatcher(TopicKey topicKey, _stopTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); _cancellationToken = _stopTokenSource.Token; ConsumeTask = Task.Run(Consume, _cancellationToken); + ConsumeTask.Forget(); } internal Task ConsumeTask { get; } diff --git a/src/UiPath.Caching/Broadcast/Redis/RedisStreamHealthMaintainer.cs b/src/UiPath.Caching/Broadcast/Redis/RedisStreamHealthMaintainer.cs index edfc6ce5..1233e0fd 100644 --- a/src/UiPath.Caching/Broadcast/Redis/RedisStreamHealthMaintainer.cs +++ b/src/UiPath.Caching/Broadcast/Redis/RedisStreamHealthMaintainer.cs @@ -62,6 +62,7 @@ public Task StartAsync(CancellationToken cancellationToken) } Initialize(); Task = Task.Run(Start, _cancellationToken); + Task.Forget(); return Task.CompletedTask; } @@ -230,8 +231,8 @@ private async Task CheckStreamWithGroupsAsync(StreamContext context, DateTimeOff { var groupInfos = await Database.StreamGroupInfoAsync(context.StreamKey, CommandFlags.DemandMaster).ConfigureAwait(false); var transaction = Database.CreateTransaction(); - _ = transaction.HashSetAsync(context.QuarantineKey, [], CommandFlags.DemandMaster).ConfigureAwait(false); - _ = transaction.KeyExpireAsync(context.QuarantineKey, _streamOptions.MaintainerQuarantineInterval.Multiply(10), CommandFlags.DemandMaster).ConfigureAwait(false); + transaction.HashSetAsync(context.QuarantineKey, [], CommandFlags.DemandMaster).Forget(); + transaction.KeyExpireAsync(context.QuarantineKey, _streamOptions.MaintainerQuarantineInterval.Multiply(10), CommandFlags.DemandMaster).Forget(); var success = await transaction.ExecuteAsync(); if (!success) { diff --git a/src/UiPath.Caching/Broadcast/Redis/RedisStreamSubjectWriter.cs b/src/UiPath.Caching/Broadcast/Redis/RedisStreamSubjectWriter.cs index a880806e..e9bb63ef 100644 --- a/src/UiPath.Caching/Broadcast/Redis/RedisStreamSubjectWriter.cs +++ b/src/UiPath.Caching/Broadcast/Redis/RedisStreamSubjectWriter.cs @@ -55,6 +55,7 @@ public RedisStreamSubjectWriter( _connectionState.OnReconnected += OnConnectionRecovered; _connectionState.OnConnectionRestored += OnConnectionRecovered; FetchTask = Task.Run(FetchLoop, _cancelationToken); + FetchTask.Forget(); } internal Task FetchTask { get; } diff --git a/src/UiPath.Caching/CacheMemoryMonitor.cs b/src/UiPath.Caching/CacheMemoryMonitor.cs index e7d9efc6..af7ecca7 100644 --- a/src/UiPath.Caching/CacheMemoryMonitor.cs +++ b/src/UiPath.Caching/CacheMemoryMonitor.cs @@ -24,6 +24,7 @@ public CacheMemoryMonitor(string name, _timer = new PeriodicTimer(statisticsFlushInterval); _cancelationToken = _cancellationTokenSource.Token; MonitorTask = Task.Run(StartMonitor, _cancelationToken); + MonitorTask.Forget(); } internal Task MonitorTask { get; } diff --git a/src/UiPath.Caching/Config/RedisCollectionExtensions.cs b/src/UiPath.Caching/Config/RedisCollectionExtensions.cs index d3c40588..967a5369 100644 --- a/src/UiPath.Caching/Config/RedisCollectionExtensions.cs +++ b/src/UiPath.Caching/Config/RedisCollectionExtensions.cs @@ -91,6 +91,7 @@ public static ICachingBuilder AddIRedisPlannedMaintenance(this ICachingBuilder b { builder.Services.TryAddSingleton(); builder.Services.TryAddSingleton(sp => sp.GetRequiredService()); + builder.Services.TryAddSingleton(sp => sp.GetRequiredService()); if (builder.Enabled) { builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton(sp => sp.GetRequiredService())); diff --git a/src/UiPath.Caching/PublicAPI.Unshipped.txt b/src/UiPath.Caching/PublicAPI.Unshipped.txt index acb709d3..a009ab74 100644 --- a/src/UiPath.Caching/PublicAPI.Unshipped.txt +++ b/src/UiPath.Caching/PublicAPI.Unshipped.txt @@ -1,8 +1,14 @@ #nullable enable +UiPath.Caching.Redis.IRedisConfigurationOptionsProvider.ReapplyDerivedBounds(StackExchange.Redis.ConfigurationOptions! configuration) -> void UiPath.Caching.Redis.IRedisConnector.GetPrimaries() -> System.Collections.Generic.IEnumerable! UiPath.Caching.Redis.IRedisConnector.ServerMaintenance -> System.EventHandler? +UiPath.Caching.Redis.RedisConfigurationOptionsProvider.ReapplyDerivedBounds(StackExchange.Redis.ConfigurationOptions! configuration) -> void UiPath.Caching.Redis.RedisConnectionOptions.MaintenanceNotifications.get -> UiPath.Caching.Redis.RedisMaintenanceNotifications? UiPath.Caching.Redis.RedisConnectionOptions.MaintenanceNotifications.set -> void +UiPath.Caching.Redis.RedisConnectionOptions.MaintenanceRelaxedTimeout.get -> System.TimeSpan? +UiPath.Caching.Redis.RedisConnectionOptions.MaintenanceRelaxedTimeout.set -> void +UiPath.Caching.Redis.RedisConnectionOptions.MaintenanceRelaxedWindowMax.get -> System.TimeSpan? +UiPath.Caching.Redis.RedisConnectionOptions.MaintenanceRelaxedWindowMax.set -> void UiPath.Caching.Redis.RedisConnector.GetPrimaries() -> System.Collections.Generic.IEnumerable! UiPath.Caching.Redis.RedisConnector.ServerMaintenance -> System.EventHandler? UiPath.Caching.Redis.RedisMaintenanceNotifications @@ -10,3 +16,4 @@ UiPath.Caching.Redis.RedisMaintenanceNotifications.Auto = 1 -> UiPath.Caching.Re UiPath.Caching.Redis.RedisMaintenanceNotifications.Disabled = 0 -> UiPath.Caching.Redis.RedisMaintenanceNotifications UiPath.Caching.Redis.RedisMaintenanceNotifications.Required = 2 -> UiPath.Caching.Redis.RedisMaintenanceNotifications UiPath.Caching.Redis.RedisPlannedMaintenance.RedisPlannedMaintenance(UiPath.Caching.Telemetry.ICachingTelemetryProvider! telemetryProvider, UiPath.Caching.Redis.IRedisConnector! redisConnector, UiPath.Caching.Redis.IRedisConfigurationOptionsProvider! redisConfigurationOptionsProvider, UiPath.Caching.Redis.IConnectionMultiplexerFactory! connectionMultiplexerFactory, Microsoft.Extensions.Logging.ILogger! logger, Microsoft.Extensions.Options.IOptions! options, System.Collections.Generic.IEnumerable? configurators, System.TimeProvider! clock) -> void +UiPath.Caching.Redis.RedisPlannedMaintenance.SuggestedTimeout.get -> System.TimeSpan? diff --git a/src/UiPath.Caching/Redis/IRedisConfigurationOptionsProvider.cs b/src/UiPath.Caching/Redis/IRedisConfigurationOptionsProvider.cs index b0ddd496..d3cba51b 100644 --- a/src/UiPath.Caching/Redis/IRedisConfigurationOptionsProvider.cs +++ b/src/UiPath.Caching/Redis/IRedisConfigurationOptionsProvider.cs @@ -3,4 +3,7 @@ namespace UiPath.Caching.Redis; public interface IRedisConfigurationOptionsProvider { ConfigurationOptions GetConfiguration(); + + /// Derives bounds from other settings once configurators have run. + void ReapplyDerivedBounds(ConfigurationOptions configuration) => _ = configuration; } diff --git a/src/UiPath.Caching/Redis/RedisCache.cs b/src/UiPath.Caching/Redis/RedisCache.cs index 39c72dbe..659b2b0b 100644 --- a/src/UiPath.Caching/Redis/RedisCache.cs +++ b/src/UiPath.Caching/Redis/RedisCache.cs @@ -535,17 +535,17 @@ private async ValueTask SetInternalAsync(KeyValuePair[] k { if (_cacheNullValues && expiration > TimeSpan.Zero) { - _ = transaction.StringSetAsync(redisKey, RedisValue.EmptyString, expiration, When.Always, CommandFlags.DemandMaster); + transaction.StringSetAsync(redisKey, RedisValue.EmptyString, expiration, When.Always, CommandFlags.DemandMaster).Forget(); } else { - _ = transaction.KeyDeleteAsync(redisKey, CommandFlags.DemandMaster); + transaction.KeyDeleteAsync(redisKey, CommandFlags.DemandMaster).Forget(); } } else { var serialized = SerializeValue(value); - _ = transaction.StringSetAsync(redisKey, serialized, expiration, When.Always, CommandFlags.DemandMaster); + transaction.StringSetAsync(redisKey, serialized, expiration, When.Always, CommandFlags.DemandMaster).Forget(); } } @@ -820,34 +820,36 @@ private async ValueTask RemoveAsync(CacheKey[] cacheKeys, RedisKey[] re ICacheEntry ret = DefaultEntry(); try { - var transaction = Database.CreateTransaction(); - var valueTask = transaction.StringGetAsync(redisKey, CommandFlags.PreferReplica).ConfigureAwait(false); - ConfiguredTaskAwaitable? expireTimeTask = default; - ConfiguredTaskAwaitable? ttlTask = default; - if (_supportsExpireTime) - { - expireTimeTask = transaction.KeyExpireTimeAsync(redisKey, CommandFlags.PreferReplica).ConfigureAwait(false); - } - else - { - ttlTask = transaction.KeyTimeToLiveAsync(redisKey, CommandFlags.PreferReplica).ConfigureAwait(false); - } - - var transactionResult = await _read.ExecuteAsync(async token => + // Per attempt, since a retry cannot re-run a drained transaction; boxed so the pipeline stays bool. + StrongBox<(RedisValue Value, DateTimeOffset? Expiration)>? read = null; + var committed = await _read.ExecuteAsync(async token => { token.ThrowIfCancellationRequested(); - return await transaction.ExecuteAsync(CommandFlags.PreferReplica).ConfigureAwait(false); + var transaction = Database.CreateTransaction(); + var valueTask = transaction.StringGetAsync(redisKey, CommandFlags.PreferReplica); + var expirationTask = FetchExpirationAsync(transaction, redisKey); + + // Observed now: an uncommitted transaction leaves them unawaited. + valueTask.Forget(); + expirationTask.Forget(); + if (!await transaction.ExecuteAsync(CommandFlags.PreferReplica).ConfigureAwait(false)) + { + return false; + } + + Interlocked.CompareExchange(ref read, new((await valueTask.ConfigureAwait(false), await expirationTask.ConfigureAwait(false))), null); + return true; }, default, token).ConfigureAwait(false); - if (!transactionResult) + if (!committed || read is null) { operation.Stop(); return ret; } - var value = await valueTask; + var (value, expiration) = read.Value; _auditKeySize?.Invoke(Logged(cacheKey, redisKey, typeof(T)), value); var (found, deserialized) = InterpretReadResult(value); @@ -857,9 +859,6 @@ private async ValueTask RemoveAsync(CacheKey[] cacheKeys, RedisKey[] re return ret; } - DateTimeOffset? expiration = _supportsExpireTime - ? (DateTimeOffset?)await expireTimeTask!.Value - : Clock.ToDateTimeOffset(await ttlTask!.Value); ret = _cacheEntryFactory.Create(deserialized, Clock.ToDateTimeOffset(expiration)); operation.Stop(); } @@ -895,25 +894,33 @@ private async ValueTask RemoveAsync(CacheKey[] cacheKeys, RedisKey[] re try { ThrowIfCrossSlot(keys, redisKeys, typeof(T), nameof(GetCacheEntriesAsync)); - var transaction = Database.CreateTransaction(); - var mgetTask = transaction.StringGetAsync(redisKeys, CommandFlags.PreferReplica).ConfigureAwait(false); - var (expireTimeTasks, ttlTasks) = StartExpirationFetches(transaction, redisKeys); - - var transactionResult = await _read.ExecuteAsync(async token => + StrongBox<(RedisValue[] Values, DateTimeOffset?[] Expirations)>? read = null; + var committed = await _read.ExecuteAsync(async token => { token.ThrowIfCancellationRequested(); - return await transaction.ExecuteAsync(CommandFlags.PreferReplica).ConfigureAwait(false); + var transaction = Database.CreateTransaction(); + var mgetTask = transaction.StringGetAsync(redisKeys, CommandFlags.PreferReplica); + var expirationTasks = redisKeys.Select(k => FetchExpirationAsync(transaction, k)).ToArray(); + mgetTask.Forget(); + Array.ForEach(expirationTasks, TaskObservation.Forget); + if (!await transaction.ExecuteAsync(CommandFlags.PreferReplica).ConfigureAwait(false)) + { + return false; + } + + Interlocked.CompareExchange(ref read, new((await mgetTask.ConfigureAwait(false), await Task.WhenAll(expirationTasks).ConfigureAwait(false))), null); + return true; }, default, token).ConfigureAwait(false); - if (!transactionResult) + if (!committed || read is null) { operation.Stop(); return retValues; } - var values = await mgetTask; + var (values, expirations) = read.Value; for (int i = 0; i < redisKeys.Length; i++) { var value = values[i]; @@ -923,13 +930,10 @@ private async ValueTask RemoveAsync(CacheKey[] cacheKeys, RedisKey[] re { continue; } - DateTimeOffset? expiration = _supportsExpireTime - ? (DateTimeOffset?)await expireTimeTasks![i] - : Clock.ToDateTimeOffset(await ttlTasks![i]); reads[i].Hit = true; retValues[i] = new KeyValuePair>( keys[i], - _cacheEntryFactory.Create(deserialized, Clock.ToDateTimeOffset(expiration))); + _cacheEntryFactory.Create(deserialized, Clock.ToDateTimeOffset(expirations[i]))); } operation.Stop(); } @@ -954,20 +958,14 @@ private async ValueTask RemoveAsync(CacheKey[] cacheKeys, RedisKey[] re return retValues; } - private (ConfiguredTaskAwaitable[]? ExpireTimeTasks, ConfiguredTaskAwaitable[]? TtlTasks) - StartExpirationFetches(ITransaction transaction, RedisKey[] redisKeys) + private async Task FetchExpirationAsync(ITransaction transaction, RedisKey redisKey) { if (_supportsExpireTime) { - var expireTimeTasks = redisKeys - .Select(k => transaction.KeyExpireTimeAsync(k, CommandFlags.PreferReplica).ConfigureAwait(false)) - .ToArray(); - return (expireTimeTasks, null); + return await transaction.KeyExpireTimeAsync(redisKey, CommandFlags.PreferReplica).ConfigureAwait(false); } - var ttlTasks = redisKeys - .Select(k => transaction.KeyTimeToLiveAsync(k, CommandFlags.PreferReplica).ConfigureAwait(false)) - .ToArray(); - return (null, ttlTasks); + + return Clock.ToDateTimeOffset(await transaction.KeyTimeToLiveAsync(redisKey, CommandFlags.PreferReplica).ConfigureAwait(false)); } private ICacheEntry DefaultEntry() => diff --git a/src/UiPath.Caching/Redis/RedisConfigurationOptionsProvider.cs b/src/UiPath.Caching/Redis/RedisConfigurationOptionsProvider.cs index a07d88ae..409cff74 100644 --- a/src/UiPath.Caching/Redis/RedisConfigurationOptionsProvider.cs +++ b/src/UiPath.Caching/Redis/RedisConfigurationOptionsProvider.cs @@ -4,6 +4,8 @@ namespace UiPath.Caching.Redis; public class RedisConfigurationOptionsProvider(ILoggerFactory loggerFactory, IOptions optionsAccessor) : IRedisConfigurationOptionsProvider { + private const double MaxMaintenanceSeconds = 600d; + private readonly RedisConnectionOptions _options = optionsAccessor.Value; public ConfigurationOptions GetConfiguration() @@ -28,6 +30,8 @@ public ConfigurationOptions GetConfiguration() LoggerFactory = loggerFactory, }; ApplyMaintenanceNotifications(supplied); + ApplyMaintenanceRelaxedWindow(supplied); + ApplyBacklogPolicy(supplied); return supplied; } @@ -45,6 +49,7 @@ public ConfigurationOptions GetConfiguration() } ApplyMaintenanceNotifications(config); + ApplyMaintenanceRelaxedWindow(config); if (_options.HeartbeatConsistencyChecks.HasValue) { @@ -56,13 +61,52 @@ public ConfigurationOptions GetConfiguration() config.HeartbeatInterval = _options.HeartbeatInterval.Value; } - if (_options.FailFastBacklogPolicy.GetValueOrDefault()) + ApplyBacklogPolicy(config); + + return config; + } + + /// + public void ReapplyDerivedBounds(ConfigurationOptions configuration) + { + // ToString lists only assigned keys, so a present key means something set it. + var rendered = configuration.ToString(); + if (HasKey(rendered, "maintRelaxedTimeout")) { - config.BacklogPolicy = BacklogPolicy.FailFast; + return; } - return config; + // Nothing to derive from; the async timeout follows the sync one when unset. + if (string.IsNullOrWhiteSpace(_options.ConnectionString) && !HasKey(rendered, "asyncTimeout") && !HasKey(rendered, "syncTimeout")) + { + return; + } + +#pragma warning disable SER010 // Server-native maintenance notifications are for evaluation purposes only + configuration.MaintenanceRelaxedTimeout = DeriveRelaxedTimeout(configuration.AsyncTimeout); +#pragma warning restore SER010 + } + + // The client renders whole seconds, and a zero window maximum turns relaxation off. + private static TimeSpan WholeSeconds(TimeSpan value) => + TimeSpan.FromSeconds(Math.Clamp(Math.Ceiling(value.TotalSeconds), 1d, MaxMaintenanceSeconds)); + + private static TimeSpan DeriveRelaxedTimeout(int asyncTimeoutMilliseconds) => + WholeSeconds(TimeSpan.FromMilliseconds(asyncTimeoutMilliseconds * 2L)); + + private static bool HasKey(string connectionString, string key) => + connectionString.Split(',').Any(part => + part.Split('=', 2) is [var name, _] && name.Trim().Equals(key, StringComparison.OrdinalIgnoreCase)); + + private void ApplyBacklogPolicy(ConfigurationOptions config) + { + // Only an explicit false restores queuing. + if (_options.FailFastBacklogPolicy is not false) + { + config.BacklogPolicy = BacklogPolicy.FailFast; + } } + private void ApplyMaintenanceNotifications(ConfigurationOptions config) { if (_options.MaintenanceNotifications is not { } notifications) @@ -83,4 +127,20 @@ private void ApplyMaintenanceNotifications(ConfigurationOptions config) #pragma warning restore SER010 } + private void ApplyMaintenanceRelaxedWindow(ConfigurationOptions config) + { + // Supplied values only; ReapplyDerivedBounds derives after the configurators. +#pragma warning disable SER010 // Server-native maintenance notifications are for evaluation purposes only + if (_options.MaintenanceRelaxedTimeout is { } relaxedTimeout) + { + config.MaintenanceRelaxedTimeout = WholeSeconds(relaxedTimeout); + } + + // Unset, the client derives it from the relaxed timeout. + if (_options.MaintenanceRelaxedWindowMax is { } windowMax) + { + config.MaintenanceRelaxedWindowMax = WholeSeconds(windowMax); + } +#pragma warning restore SER010 + } } diff --git a/src/UiPath.Caching/Redis/RedisConnectionConfigurators.cs b/src/UiPath.Caching/Redis/RedisConnectionConfigurators.cs index 27c76a3a..2e9ae35f 100644 --- a/src/UiPath.Caching/Redis/RedisConnectionConfigurators.cs +++ b/src/UiPath.Caching/Redis/RedisConnectionConfigurators.cs @@ -2,17 +2,18 @@ namespace UiPath.Caching.Redis; internal static class RedisConnectionConfigurators { - public static async ValueTask ApplyAsync(ConfigurationOptions configuration, IEnumerable? configurators, CancellationToken cancellationToken) + public static async ValueTask ApplyAsync(ConfigurationOptions configuration, IEnumerable? configurators, IRedisConfigurationOptionsProvider optionsProvider, CancellationToken cancellationToken) { - if (configurators is null) + if (configurators is not null) { - return; + foreach (var configurator in configurators) + { + cancellationToken.ThrowIfCancellationRequested(); + await configurator.ConfigureAsync(configuration, cancellationToken).ConfigureAwait(false); + } } - foreach (var configurator in configurators) - { - cancellationToken.ThrowIfCancellationRequested(); - await configurator.ConfigureAsync(configuration, cancellationToken).ConfigureAwait(false); - } + // Last, so derived bounds follow the final async timeout. + optionsProvider.ReapplyDerivedBounds(configuration); } } diff --git a/src/UiPath.Caching/Redis/RedisConnectionOptions.cs b/src/UiPath.Caching/Redis/RedisConnectionOptions.cs index a8666c4d..13592a45 100644 --- a/src/UiPath.Caching/Redis/RedisConnectionOptions.cs +++ b/src/UiPath.Caching/Redis/RedisConnectionOptions.cs @@ -25,6 +25,12 @@ public class RedisConnectionOptions /// Ask the server for advance notice of maintenance; null leaves the client's own default. public RedisMaintenanceNotifications? MaintenanceNotifications { get; set; } + /// Command timeout during an announced window; unset derives twice the async timeout. + public TimeSpan? MaintenanceRelaxedTimeout { get; set; } + + /// Caps an announced window; unset lets the client derive it. + public TimeSpan? MaintenanceRelaxedWindowMax { get; set; } + public int PlannedMaintenanceConnectionRetryCount { get; set; } = 5; public TimeSpan PlannedMaintenanceConnectionRetryDelay { get; set; } = TimeSpan.FromSeconds(5); @@ -52,7 +58,7 @@ public class RedisConnectionOptions public TimeSpan StaleEndpointScanInterval { get; set; } = TimeSpan.FromSeconds(30); - public bool? FailFastBacklogPolicy { get; set; } + public bool? FailFastBacklogPolicy { get; set; } = true; public bool ProfilerEnabled { get; set; } diff --git a/src/UiPath.Caching/Redis/RedisConnectionWarmup.cs b/src/UiPath.Caching/Redis/RedisConnectionWarmup.cs index 8a4042fd..71df14ca 100644 --- a/src/UiPath.Caching/Redis/RedisConnectionWarmup.cs +++ b/src/UiPath.Caching/Redis/RedisConnectionWarmup.cs @@ -10,7 +10,7 @@ internal sealed class RedisConnectionWarmup(IRedisConnector connector, ICachingT public Task StartAsync(CancellationToken cancellationToken) { - _ = Task.Run(() => WarmUpAsync(_cancellationTokenSource.Token), _cancellationTokenSource.Token); + Task.Run(() => WarmUpAsync(_cancellationTokenSource.Token), _cancellationTokenSource.Token).Forget(); return Task.CompletedTask; } diff --git a/src/UiPath.Caching/Redis/RedisConnector.cs b/src/UiPath.Caching/Redis/RedisConnector.cs index 9c2f4fa1..bc9bef9e 100644 --- a/src/UiPath.Caching/Redis/RedisConnector.cs +++ b/src/UiPath.Caching/Redis/RedisConnector.cs @@ -410,7 +410,7 @@ private void ForceReconnect(Lazy> current) return; } - _ = Task.Run(async () => + Task.Run(async () => { if (Interlocked.CompareExchange(ref _reconnecting, 1, 0) != 0) { @@ -456,7 +456,7 @@ private void ForceReconnect(Lazy> current) { Interlocked.Exchange(ref _reconnecting, 0); } - }); + }).Forget(); } private async Task CloseAndDisposeAsync(Task multiplexerTask) @@ -901,7 +901,7 @@ private void OnInternalConnectionFailed(object? sender, ConnectionFailedEventArg private async ValueTask CreateMultiplexerAsync(CancellationToken cancellationToken) { var configuration = _redisConfigurationOptionsProvider.GetConfiguration(); - await RedisConnectionConfigurators.ApplyAsync(configuration, _configurators, cancellationToken).ConfigureAwait(false); + await RedisConnectionConfigurators.ApplyAsync(configuration, _configurators, _redisConfigurationOptionsProvider, cancellationToken).ConfigureAwait(false); return await _connectionMultiplexerFactory.CreateAsync(configuration, cancellationToken).ConfigureAwait(false); } diff --git a/src/UiPath.Caching/Redis/RedisHashCache.cs b/src/UiPath.Caching/Redis/RedisHashCache.cs index 1d258dba..add14625 100644 --- a/src/UiPath.Caching/Redis/RedisHashCache.cs +++ b/src/UiPath.Caching/Redis/RedisHashCache.cs @@ -341,7 +341,7 @@ public async ValueTask SetMetadataAsync(CacheKey cacheKey, IDictionary< token.ThrowIfCancellationRequested(); var transaction = Database.CreateTransaction(); transaction.AddCondition(Condition.KeyExists(redisKey)); - _ = transaction.HashSetAsync(redisKey, KnownFieldNames.MetadataKey, metadataValue, When.Always, CommandFlags.DemandMaster); + transaction.HashSetAsync(redisKey, KnownFieldNames.MetadataKey, metadataValue, When.Always, CommandFlags.DemandMaster).Forget(); return await transaction.ExecuteAsync(CommandFlags.DemandMaster).ConfigureAwait(false); }, default, @@ -366,7 +366,7 @@ public async ValueTask SetMetadataAsync(CacheKey cacheKey, IDictionary< token.ThrowIfCancellationRequested(); var transaction = Database.CreateTransaction(); transaction.AddCondition(Condition.KeyExists(redisKey)); - _ = transaction.HashSetAsync(redisKey, KnownFieldNames.MetadataKey, RedisValue.EmptyString, When.Always, CommandFlags.DemandMaster); + transaction.HashSetAsync(redisKey, KnownFieldNames.MetadataKey, RedisValue.EmptyString, When.Always, CommandFlags.DemandMaster).Forget(); return await transaction.ExecuteAsync(CommandFlags.DemandMaster).ConfigureAwait(false); }, default, @@ -403,10 +403,10 @@ private static void QueueExpirationUpdate(ITransaction transaction, RedisKey red { if (expiration != DateTimeOffset.MaxValue) { - _ = transaction.KeyExpireAsync(redisKey, expiration.UtcDateTime, CommandFlags.DemandMaster | CommandFlags.FireAndForget).ConfigureAwait(false); + transaction.KeyExpireAsync(redisKey, expiration.UtcDateTime, CommandFlags.DemandMaster | CommandFlags.FireAndForget).Forget(); return; } - _ = transaction.KeyPersistAsync(redisKey, CommandFlags.DemandMaster | CommandFlags.FireAndForget).ConfigureAwait(false); + transaction.KeyPersistAsync(redisKey, CommandFlags.DemandMaster | CommandFlags.FireAndForget).Forget(); } private static void ValidateForWrite(IDictionary values) @@ -624,17 +624,17 @@ private void QueueMetadataWrite(ITransaction transaction, RedisKey redisKey, IDi transaction.AddCondition(Condition.KeyExists(redisKey)); } var hashEntries = new[] { new HashEntry(KnownFieldNames.MetadataKey, _serializer.Serialize(metadata)) }; - _ = transaction.HashSetAsync(redisKey, hashEntries, CommandFlags.DemandMaster).ConfigureAwait(false); + transaction.HashSetAsync(redisKey, hashEntries, CommandFlags.DemandMaster).Forget(); return; } if (_cacheNullValues) { transaction.AddCondition(Condition.KeyExists(redisKey)); var entries = new[] { new HashEntry(KnownFieldNames.MetadataKey, RedisValue.EmptyString) }; - _ = transaction.HashSetAsync(redisKey, entries, CommandFlags.DemandMaster).ConfigureAwait(false); + transaction.HashSetAsync(redisKey, entries, CommandFlags.DemandMaster).Forget(); return; } - _ = transaction.HashDeleteAsync(redisKey, new RedisValue(KnownFieldNames.MetadataKey), CommandFlags.DemandMaster).ConfigureAwait(false); + transaction.HashDeleteAsync(redisKey, new RedisValue(KnownFieldNames.MetadataKey), CommandFlags.DemandMaster).Forget(); } private ValueTask SetCoreAsync(CacheKey cacheKey, IDictionary values, DateTimeOffset effective, CancellationToken token) @@ -666,37 +666,40 @@ private RedisValue SerializeFieldValue(T? value) private async ValueTask>> GetCacheEntryForKeyAsync(CacheKey cacheKey, RedisKey redisKey, CancellationToken token) { token.ThrowIfCancellationRequested(); - var transaction = Database.CreateTransaction(); - var hashEntriesTask = transaction.HashGetAllAsync(redisKey, CommandFlags.PreferReplica).ConfigureAwait(false); - ConfiguredTaskAwaitable? expireTimeTask = default; - ConfiguredTaskAwaitable? expireTimeToLiveTask = default; - if (_supportsExpireTime) - { - expireTimeTask = transaction.KeyExpireTimeAsync(redisKey, CommandFlags.PreferReplica).ConfigureAwait(false); - } - else - { - expireTimeToLiveTask = transaction.KeyTimeToLiveAsync(redisKey, CommandFlags.PreferReplica).ConfigureAwait(false); - } - - var transactionResult = await _read.ExecuteAsync( async token => + // Per attempt, since a retry cannot re-run a drained transaction; boxed so the pipeline stays bool. + StrongBox<(HashEntry[] Entries, DateTimeOffset? Expiration)>? read = null; + var committed = await _read.ExecuteAsync(async token => { token.ThrowIfCancellationRequested(); - return await transaction.ExecuteAsync(CommandFlags.PreferReplica).ConfigureAwait(false); + var transaction = Database.CreateTransaction(); + var hashEntriesTask = transaction.HashGetAllAsync(redisKey, CommandFlags.PreferReplica); + Task? expireTimeTask = _supportsExpireTime ? transaction.KeyExpireTimeAsync(redisKey, CommandFlags.PreferReplica) : null; + Task? expireTimeToLiveTask = _supportsExpireTime ? null : transaction.KeyTimeToLiveAsync(redisKey, CommandFlags.PreferReplica); + + // Observed now: an uncommitted transaction leaves them unawaited. + hashEntriesTask.Forget(); + expireTimeTask?.Forget(); + expireTimeToLiveTask?.Forget(); + if (!await transaction.ExecuteAsync(CommandFlags.PreferReplica).ConfigureAwait(false)) + { + return false; + } + + var entries = await hashEntriesTask.ConfigureAwait(false); + var expiration = expireTimeTask is not null + ? (DateTimeOffset?)await expireTimeTask.ConfigureAwait(false) + : Clock.ToDateTimeOffset(await expireTimeToLiveTask!.ConfigureAwait(false)); + Interlocked.CompareExchange(ref read, new((entries, expiration)), null); + return true; }, default, token).ConfigureAwait(false); - if (!transactionResult) + if (!committed || read is null) { throw new InvalidOperationException("Unable to read from redis"); } - var hashEntries = await hashEntriesTask; - var expireTime = _supportsExpireTime - ? (DateTimeOffset?)await expireTimeTask!.Value - : Clock.ToDateTimeOffset(await expireTimeToLiveTask!.Value); - - return ParseCacheEntry(cacheKey, redisKey, hashEntries, expireTime); + return ParseCacheEntry(cacheKey, redisKey, read.Value.Entries, read.Value.Expiration); } [SuppressMessage("SonarLint.Rule", "S3776")] @@ -993,10 +996,10 @@ private async ValueTask SetInnerAsync(RedisKey redisKey, HashEntry[] ha var transaction = Database.CreateTransaction(); if (setOption == HashCacheSetOption.KeyReplace) { - _ = transaction.KeyDeleteAsync(redisKey).ConfigureAwait(false); + transaction.KeyDeleteAsync(redisKey).Forget(); } - _ = transaction.HashSetAsync(redisKey, hashEntries, CommandFlags.DemandMaster).ConfigureAwait(false); + transaction.HashSetAsync(redisKey, hashEntries, CommandFlags.DemandMaster).Forget(); if (expiration != DateTimeOffset.MaxValue) { await transaction.KeyExpireAsync(redisKey, expiration.UtcDateTime, CommandFlags.DemandMaster | CommandFlags.FireAndForget).ConfigureAwait(false); @@ -1004,7 +1007,7 @@ private async ValueTask SetInnerAsync(RedisKey redisKey, HashEntry[] ha else if (setOption == HashCacheSetOption.HashReplace) { // The key survives a HashReplace, and so would a TTL an earlier write gave it. - _ = transaction.KeyPersistAsync(redisKey, CommandFlags.DemandMaster | CommandFlags.FireAndForget).ConfigureAwait(false); + transaction.KeyPersistAsync(redisKey, CommandFlags.DemandMaster | CommandFlags.FireAndForget).Forget(); } ret = await _write.ExecuteAsync(async token => diff --git a/src/UiPath.Caching/Redis/RedisPlannedMaintenance.cs b/src/UiPath.Caching/Redis/RedisPlannedMaintenance.cs index 5f07ff8a..9efdff22 100644 --- a/src/UiPath.Caching/Redis/RedisPlannedMaintenance.cs +++ b/src/UiPath.Caching/Redis/RedisPlannedMaintenance.cs @@ -1,15 +1,19 @@ using System.Globalization; using Microsoft.Extensions.Hosting; using StackExchange.Redis.Maintenance; +using UiPath.Caching.Policies; using UiPath.Caching.Telemetry; namespace UiPath.Caching.Redis; [ExcludeFromCodeCoverage(Justification = "Wires up StackExchange.Redis ServerMaintenanceEvent — exercised only by real Azure Cache for Redis planned-maintenance notifications.")] -public sealed class RedisPlannedMaintenance : IRedisPlannedMaintenance, IHostedService +public sealed class RedisPlannedMaintenance : IRedisPlannedMaintenance, IHostedService, IDisruptionState { /// How long a notification stays recognisable as one already recorded, and so what bounds the set. - private static readonly TimeSpan SeenRetention = TimeSpan.FromSeconds(30); + private static readonly TimeSpan MinSeenRetention = TimeSpan.FromSeconds(30); +#pragma warning disable SER010 // Server-native maintenance notifications are for evaluation purposes only + private static readonly ConfigurationOptions MaintenanceDefaults = new(); +#pragma warning restore SER010 private readonly ICachingTelemetryProvider _telemetryProvider; private readonly IRedisConnector _redisConnector; @@ -26,10 +30,34 @@ public sealed class RedisPlannedMaintenance : IRedisPlannedMaintenance, IHostedS private readonly object _stateLock = new(); private readonly object _seenLock = new(); private readonly object _cancelLock = new(); + + // Taken first: a notice adopts bounds and updates its window as one step. + private readonly object _noticeLock = new(); private readonly Queue<(string Raw, long At)> _seen = new(); private readonly HashSet _seenIdentities = new(StringComparer.Ordinal); + private readonly object _windowLock = new(); +#pragma warning disable SER010 // Server-native maintenance notifications are for evaluation purposes only + private readonly Dictionary> _outstanding = []; + + // Per live family, so a replay cannot change it after its claim expires. + private readonly Dictionary> _familyIdentities = []; +#pragma warning restore SER010 private readonly TimeProvider _clock; private readonly CancellationTokenSource _cancellationTokenSource = new(); + private CancellationTokenSource? _windowTimer; + private bool _reportedInProgress; +#pragma warning disable SER010 // Server-native maintenance notifications are for evaluation purposes only + private TimeSpan _relaxedTimeout = MaintenanceDefaults.MaintenanceRelaxedTimeout; + private TimeSpan _relaxedWindowMax = MaintenanceDefaults.MaintenanceRelaxedWindowMax; + private TimeSpan _postEventRelaxed = MaintenanceDefaults.MaintenancePostEventRelaxedDuration; + private long _seenRetentionTicks = RetentionTicks(MaintenanceDefaults.MaintenanceRelaxedWindowMax, MaintenanceDefaults.MaintenancePostEventRelaxedDuration); + private long _relaxedTimeoutTicks = MaintenanceDefaults.MaintenanceRelaxedTimeout.Ticks; +#pragma warning restore SER010 + + // Written under _windowLock, read without it on every Redis operation. + private long _openUntil = long.MinValue; + private string? _adoptedConfiguration; + private long? _disconnectedSince; private IConnectionMultiplexer? _multiplexer; private volatile bool _disposed; private bool _cancellationDisposed; @@ -72,15 +100,23 @@ public RedisPlannedMaintenance( public bool InProgress { - get => Interlocked.Read(ref _maintenanceInProgress) == 1; + get => Interlocked.Read(ref _maintenanceInProgress) == 1 || AnnouncedWindowOpen; set => Interlocked.Exchange(ref _maintenanceInProgress, value ? 1 : 0); } + /// The relaxed timeout while an announced window is open. + public TimeSpan? SuggestedTimeout => AnnouncedWindowOpen ? new TimeSpan(Volatile.Read(ref _relaxedTimeoutTicks)) : null; + + // A window plus its tail, published as one word so both bounds are read together. + private TimeSpan SeenRetention => new(Volatile.Read(ref _seenRetentionTicks)); + + private bool AnnouncedWindowOpen => _clock.GetTimestamp() < Volatile.Read(ref _openUntil); + public Task StartAsync(CancellationToken cancellationToken) { // Via the connector, so one subscription survives a ForceReconnect; the multiplexer's own would not. _redisConnector.ServerMaintenance += OnServerMaintenance; - _ = Task.Run(() => InitializeAsync(_cancellationTokenSource.Token), _cancellationTokenSource.Token); + Task.Run(() => InitializeAsync(_cancellationTokenSource.Token), _cancellationTokenSource.Token).Forget(); return Task.CompletedTask; } @@ -115,6 +151,22 @@ public void Dispose() } } +#pragma warning disable SER010 // Server-native maintenance notifications are for evaluation purposes only + private static long RetentionTicks(TimeSpan relaxedWindowMax, TimeSpan postEventRelaxed) + { + var horizon = relaxedWindowMax + postEventRelaxed; + return (horizon > MinSeenRetention ? horizon : MinSeenRetention).Ticks; + } + + private static MaintenanceNotificationType? StarterFor(MaintenanceNotificationType completion) => completion switch + { + MaintenanceNotificationType.Migrated => MaintenanceNotificationType.Migrating, + MaintenanceNotificationType.FailedOver => MaintenanceNotificationType.FailingOver, + MaintenanceNotificationType.SlotMigrated => MaintenanceNotificationType.SlotMigrating, + _ => null, + }; +#pragma warning restore SER010 + /// Stops the service reacting further; both the stop and the dispose path run it. private void StopReacting(IConnectionMultiplexer? multiplexer = null) { @@ -136,6 +188,88 @@ private void StopReacting(IConnectionMultiplexer? multiplexer = null) multiplexer.ServerMaintenanceEvent -= OnMaintenanceConnectionEvent; } + CloseAllAnnouncedWindows(); + } + + /// One transition for both routes, which can overlap. + private void ReportAggregateState() + { + lock (_windowLock) + { + var open = InProgress; + + // No start while stopping, but still close an open one. + if (open && (_disposed || Volatile.Read(ref _stopped) == 1)) + { + return; + } + + if (open == _reportedInProgress) + { + return; + } + + // Only once announced, or an end would pair with nothing. + if (TryAnnounce(open ? "Redis.MaintenanceStarted" : "Redis.MaintenanceEnded")) + { + _reportedInProgress = open; + } + } + } + + private void PublishSeenRetention() => + Volatile.Write(ref _seenRetentionTicks, RetentionTicks(_relaxedWindowMax, _postEventRelaxed)); + + private void AdoptMaintenanceBounds(ConfigurationOptions configuration, string rendered) + { + lock (_windowLock) + { + // With the bounds, so the marker always names them. + _adoptedConfiguration = rendered; +#pragma warning disable SER010 // Server-native maintenance notifications are for evaluation purposes only + _relaxedTimeout = configuration.MaintenanceRelaxedTimeout; + _relaxedWindowMax = configuration.MaintenanceRelaxedWindowMax; + _postEventRelaxed = configuration.MaintenancePostEventRelaxedDuration; +#pragma warning restore SER010 + Volatile.Write(ref _relaxedTimeoutTicks, _relaxedTimeout.Ticks); + PublishSeenRetention(); + + // Shortened bounds can move a deadline into the past. + ReevaluateWindow(); + } + } + + // The sending connection was built with the configurators, so its bounds are the real ones. + private void AdoptBoundsOf(object? sender) + { + if (sender is not IConnectionMultiplexer multiplexer) + { + return; + } + + try + { + var rendered = multiplexer.Configuration; + if (string.IsNullOrWhiteSpace(rendered)) + { + return; + } + + lock (_windowLock) + { + if (string.Equals(rendered, _adoptedConfiguration, StringComparison.Ordinal)) + { + return; + } + } + + AdoptMaintenanceBounds(ConfigurationOptions.Parse(rendered), rendered); + } + catch (Exception ex) + { + // Keep the current bounds; the notice is still recorded. + _telemetryProvider.TryTrackException(ex); + } } private void Cancel() @@ -244,7 +378,9 @@ private async Task InitializeAsync(CancellationToken cancellationToken) private async Task TryConnectAsync(CancellationToken cancellationToken) { var configuration = _redisConfigurationOptionsProvider.GetConfiguration(); - await RedisConnectionConfigurators.ApplyAsync(configuration, _configurators, cancellationToken).ConfigureAwait(false); + await RedisConnectionConfigurators.ApplyAsync(configuration, _configurators, _redisConfigurationOptionsProvider, cancellationToken).ConfigureAwait(false); + + // Not adopted: this connection relaxes nothing. var multiplexer = await _connectionMultiplexerFactory.CreateAsync(configuration, cancellationToken).ConfigureAwait(false); @@ -392,24 +528,30 @@ private void Settle(string? claim, bool recorded) private void OnServerMaintenance(object? sender, ServerMaintenanceEvent e) { - if (!TryClaim(e, out var claim)) + lock (_noticeLock) { - return; - } + // Claim first, so a copy from a retired connection cannot resize the window. + if (!TryClaim(e, out var claim)) + { + return; + } - var recorded = false; - try - { - Record(e); - recorded = true; - } - finally - { - Settle(claim, recorded); + AdoptBoundsOf(sender); + + var recorded = false; + try + { + Record(e, claim); + recorded = true; + } + finally + { + Settle(claim, recorded); + } } } - private void Record(ServerMaintenanceEvent e) + private void Record(ServerMaintenanceEvent e, string? identity) { #pragma warning disable SER010 // Server-native maintenance notifications are for evaluation purposes only switch (e) @@ -418,11 +560,11 @@ private void Record(ServerMaintenanceEvent e) OnAzureMaintenance(azureEvent); break; case PushMaintenanceEvent pushEvent: - OnPushMaintenance(pushEvent); + OnPushMaintenance(pushEvent, identity); break; default: // Recording the base properties keeps a source we do not model visible, rather than silent. - _telemetryProvider.TrackEvent( + _telemetryProvider.TryTrackEvent( "Redis.Maintenance", [ new("Source", e.GetType().Name), @@ -444,7 +586,7 @@ private void OnAzureMaintenance(AzureMaintenanceEvent azureEvent) StartConnectionProbing(); } - _telemetryProvider.TrackEvent( + _telemetryProvider.TryTrackEvent( "Redis.Maintenance", [ new("Source", nameof(AzureMaintenanceEvent)), @@ -459,11 +601,27 @@ private void OnAzureMaintenance(AzureMaintenanceEvent azureEvent) } #pragma warning disable SER010 // Server-native maintenance notifications are for evaluation purposes only - private void OnPushMaintenance(PushMaintenanceEvent pushEvent) + private void OnPushMaintenance(PushMaintenanceEvent pushEvent, string? identity) { - // Recorded, not acted on: the client handles the handoff itself, and probing force-reconnects on a - // failed write, which would fight it. - _telemetryProvider.TrackEvent( + // No probing: it force-reconnects, fighting the client's handoff. + switch (pushEvent.NotificationType) + { + case MaintenanceNotificationType.Moving: + case MaintenanceNotificationType.Migrating: + case MaintenanceNotificationType.FailingOver: + case MaintenanceNotificationType.SlotMigrating: + OpenAnnouncedWindow(pushEvent.NotificationType, pushEvent.Time, identity); + break; + case MaintenanceNotificationType.Migrated: + case MaintenanceNotificationType.FailedOver: + case MaintenanceNotificationType.SlotMigrated: + CloseAnnouncedWindow(pushEvent.NotificationType, identity); + break; + default: + break; + } + + _telemetryProvider.TryTrackEvent( "Redis.Maintenance", [ new("Source", nameof(PushMaintenanceEvent)), @@ -478,9 +636,185 @@ private void OnPushMaintenance(PushMaintenanceEvent pushEvent) ]); } #pragma warning restore SER010 + /// Announces an interval boundary, reporting a sink that refuses rather than ending the run. private bool TryAnnounce(string eventName) => _telemetryProvider.TryTrackEvent(eventName); +#pragma warning disable SER010 // Server-native maintenance notifications are for evaluation purposes only + // The announced duration is a hint; two-second windows have been seen. + private TimeSpan RemainingOf(AnnouncedWindow window, long now) => + (window.Tail ? _postEventRelaxed : WindowFor(window.Announced)) - _clock.GetElapsedTime(window.At, now); + + private TimeSpan WindowFor(TimeSpan? announced) + { + var window = announced is { } duration && duration > TimeSpan.Zero ? duration : _relaxedTimeout; + + // Floor, then cap, so the cap always wins. + if (window < _relaxedTimeout) + { + window = _relaxedTimeout; + } + + return window > _relaxedWindowMax ? _relaxedWindowMax : window; + } + + private void OpenAnnouncedWindow(MaintenanceNotificationType starter, TimeSpan? announced, string? identity) + { + lock (_windowLock) + { + // Under the lock StopAsync clears with. + if (_disposed || Volatile.Read(ref _stopped) == 1 || !AdmitToFamily(starter, identity)) + { + return; + } + + // One entry per operation, so a shorter one cannot shorten a longer. + var announcedWindow = new AnnouncedWindow(_clock.GetTimestamp(), announced, Tail: false); + if (_outstanding.TryGetValue(starter, out var family)) + { + family.Add(announcedWindow); + } + else + { + _outstanding[starter] = [announcedWindow]; + } + + ReevaluateWindow(); + } + } + + private void CloseAllAnnouncedWindows() + { + lock (_windowLock) + { + _outstanding.Clear(); + _familyIdentities.Clear(); + ReevaluateWindow(); + } + } + + private void CloseAnnouncedWindow(MaintenanceNotificationType completion, string? identity) + { + lock (_windowLock) + { + // Moving only lapses; other completions leave a tail, as the client does. + if (StarterFor(completion) is { } starter && _outstanding.TryGetValue(starter, out var family) && family.Count > 0 + && AdmitToFamily(starter, identity)) + { + // Which operation finished is unknowable, so release the one expiring first. + var now = _clock.GetTimestamp(); + var running = family.Where(window => !window.Tail).ToList(); + if (running.Count > 0) + { + var finished = running.OrderBy(window => RemainingOf(window, now)).First(); + family.Remove(finished); + + // Every live completion earns a tail, as the client relaxes on each. + if (_postEventRelaxed > TimeSpan.Zero + && RemainingOf(finished, now) > TimeSpan.Zero) + { + family.Add(new AnnouncedWindow(now, Announced: null, Tail: true)); + } + } + + if (family.Count == 0) + { + RemoveFamily(starter); + } + } + + ReevaluateWindow(); + } + } + + /// False when the family already applied this notice. + private bool AdmitToFamily(MaintenanceNotificationType starter, string? identity) + { + if (identity is null) + { + return true; + } + + if (!_familyIdentities.TryGetValue(starter, out var applied)) + { + applied = new HashSet(StringComparer.Ordinal); + _familyIdentities[starter] = applied; + } + + return applied.Add(identity); + } + + private void RemoveFamily(MaintenanceNotificationType starter) + { + _outstanding.Remove(starter); + _familyIdentities.Remove(starter); + } + + private void ReevaluateWindow() + { + var now = _clock.GetTimestamp(); + foreach (var (starter, family) in _outstanding.ToList()) + { + // Keep a lapsed operation while a live one could have its late completion taken. + var live = family.Exists(window => !window.Tail && RemainingOf(window, now) > TimeSpan.Zero); + family.RemoveAll(window => RemainingOf(window, now) <= TimeSpan.Zero && (window.Tail || !live)); + if (family.Count == 0) + { + RemoveFamily(starter); + } + } + + _windowTimer?.Cancel(); + _windowTimer?.Dispose(); + _windowTimer = null; + + var pending = _outstanding.Values.SelectMany(family => family).Select(window => RemainingOf(window, now)).Where(remaining => remaining > TimeSpan.Zero).ToList(); + + // Before reporting, which reads it through InProgress. + Volatile.Write(ref _openUntil, pending.Count == 0 ? long.MinValue : now + ToTimestampTicks(pending.Max())); + ReportAggregateState(); + + if (pending.Count == 0 || _disposed) + { + return; + } + + // A completion can be lost, so the earliest deadline is re-armed here. + var next = pending.Min(); + CancellationTokenSource timer; + try + { + timer = CancellationTokenSource.CreateLinkedTokenSource(_cancellationTokenSource.Token); + } + catch (ObjectDisposedException) + { + return; + } + + _windowTimer = timer; + + // Inline, so the timer is armed before this returns. + CloseWhenElapsedAsync(next, timer).Forget(); + } +#pragma warning restore SER010 + + private async Task CloseWhenElapsedAsync(TimeSpan next, CancellationTokenSource timer) + { + try + { + await Task.Delay(next, _clock, timer.Token).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + return; + } + + lock (_windowLock) + { + ReevaluateWindow(); + } + } + private void StartConnectionProbing() { // Under the lock Cancel takes, so the check and the transition are one step: separately, a caller could @@ -505,7 +839,9 @@ private void StartConnectionProbing() } catch (ObjectDisposedException) { + // Through the aggregate, so a started push window still gets its end. InProgress = false; + ReportAggregateState(); return; } @@ -513,12 +849,11 @@ private void StartConnectionProbing() var token = tokenSource.Token; // Never skip the delegate: its finally disposes the linked source and clears InProgress. - _ = Task.Run(() => ProbeUntilCancelledAsync(tokenSource, token), CancellationToken.None); + Task.Run(() => ProbeUntilCancelledAsync(tokenSource, token), CancellationToken.None).Forget(); } private async Task ProbeUntilCancelledAsync(CancellationTokenSource tokenSource, CancellationToken token) { - var started = false; try { // Queued after the lock was released, so StopAsync can have completed in between; a worker that @@ -529,14 +864,10 @@ private async Task ProbeUntilCancelledAsync(CancellationTokenSource tokenSource, { return; } - - // Under the same lock that cleared the guard: outside it, a shutdown landing between the two - // would let this worker announce an interval that starts after the service stopped. A refused - // announcement must not end the run -- probing is what it is for -- and leaves started false, - // so there is no end to announce either. - started = TryAnnounce("Redis.MaintenanceStarted"); } + ReportAggregateState(); + while (!token.IsCancellationRequested) { if (!await ProbeOnceAsync(token).ConfigureAwait(false)) @@ -547,34 +878,43 @@ private async Task ProbeUntilCancelledAsync(CancellationTokenSource tokenSource, } finally { - // Cleared first: left set, the CompareExchange guard would refuse every later probe run. + // Cleared first, or the CompareExchange guard refuses every later run. InProgress = false; + ReportAggregateState(); tokenSource.Dispose(); - if (started) - { - _ = TryAnnounce("Redis.MaintenanceEnded"); - } } } /// One probe and the wait after it; false once the run should stop. private async Task ProbeOnceAsync(CancellationToken token) { + Task? probeTask = null; try { - var probeTask = _redisConnector.Database.StringSetAsync("probeRedis_" + Environment.MachineName, DateTime.UtcNow.ToString(CultureInfo.InvariantCulture), expiry: TimeSpan.FromDays(1)); + probeTask = _redisConnector.Database.StringSetAsync("probeRedis_" + Environment.MachineName, DateTime.UtcNow.ToString(CultureInfo.InvariantCulture), expiry: TimeSpan.FromDays(1)); await probeTask.WaitAsync(_hangingTime, token).ConfigureAwait(false); + _disconnectedSince = null; } catch (OperationCanceledException) when (token.IsCancellationRequested) { + probeTask?.Forget(); return false; } catch (Exception ex) { + // WaitAsync stops observing the probe. + probeTask?.Forget(); + // Reporting must not cost the reconnect: that call is the whole point of probing. _telemetryProvider.TryTrackException(ex); - _redisConnector.ForceReconnect(); + + // Fail-fast rejects while the client reconnects; rebuild only if that outlasts the hanging time. + if (ex is not RedisConnectionException || _redisConnector.IsConnected || DisconnectedFor() >= _hangingTime) + { + _disconnectedSince = null; + _redisConnector.ForceReconnect(); + } } try @@ -588,4 +928,17 @@ private async Task ProbeOnceAsync(CancellationToken token) return true; } + + private TimeSpan DisconnectedFor() + { + var now = _clock.GetTimestamp(); + _disconnectedSince ??= now; + return _clock.GetElapsedTime(_disconnectedSince.Value, now); + } + + private long ToTimestampTicks(TimeSpan duration) => + (long)Math.Ceiling(duration.Ticks * (double)_clock.TimestampFrequency / TimeSpan.TicksPerSecond); + + /// What was announced and when, so later bounds can move its deadline. + private readonly record struct AnnouncedWindow(long At, TimeSpan? Announced, bool Tail); } diff --git a/src/UiPath.Caching/RehydrationCoordinator.cs b/src/UiPath.Caching/RehydrationCoordinator.cs index 7c17051d..e5aaaffc 100644 --- a/src/UiPath.Caching/RehydrationCoordinator.cs +++ b/src/UiPath.Caching/RehydrationCoordinator.cs @@ -106,7 +106,7 @@ private bool TryTriggerCore( } var reservedKeys = reserved.ToArray(); - _ = SpawnAsync(reservedKeys, policy.Rehydrate, duration, kind, rehydrateAsync, entryType); + SpawnAsync(reservedKeys, policy.Rehydrate, duration, kind, rehydrateAsync, entryType).Forget(); return true; } diff --git a/src/UiPath.Caching/TaskObservation.cs b/src/UiPath.Caching/TaskObservation.cs new file mode 100644 index 00000000..03dc54df --- /dev/null +++ b/src/UiPath.Caching/TaskObservation.cs @@ -0,0 +1,12 @@ +namespace UiPath.Caching; + +internal static class TaskObservation +{ + /// Observes the failure of a task nothing awaits. + public static void Forget(this Task task) => + _ = task.ContinueWith( + static failed => _ = failed.Exception, + CancellationToken.None, + TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); +} diff --git a/tests/UiPath.Caching.Tests/Fakes/RecordingResiliencePipeline.cs b/tests/UiPath.Caching.Tests/Fakes/RecordingResiliencePipeline.cs new file mode 100644 index 00000000..027b2950 --- /dev/null +++ b/tests/UiPath.Caching.Tests/Fakes/RecordingResiliencePipeline.cs @@ -0,0 +1,30 @@ +using UiPath.Caching.Policies; + +namespace UiPath.Caching.Tests.Fakes; + +/// Runs the callback once, recording its result type. +internal sealed class RecordingResiliencePipeline : IResiliencePipeline +{ + private readonly List _resultTypes = []; + + public IReadOnlyList ResultTypes + { + get + { + lock (_resultTypes) + { + return [.. _resultTypes]; + } + } + } + + public ValueTask ExecuteAsync(Func> callback, TResult defaultValue, CancellationToken cancellationToken = default) + { + lock (_resultTypes) + { + _resultTypes.Add(typeof(TResult)); + } + + return callback(cancellationToken); + } +} diff --git a/tests/UiPath.Caching.Tests/Fakes/RetryOnceResiliencePipeline.cs b/tests/UiPath.Caching.Tests/Fakes/RetryOnceResiliencePipeline.cs new file mode 100644 index 00000000..a80d0904 --- /dev/null +++ b/tests/UiPath.Caching.Tests/Fakes/RetryOnceResiliencePipeline.cs @@ -0,0 +1,20 @@ +using StackExchange.Redis; +using UiPath.Caching.Policies; + +namespace UiPath.Caching.Tests.Fakes; + +/// Replays the callback once after it throws. +internal sealed class RetryOnceResiliencePipeline : IResiliencePipeline +{ + public async ValueTask ExecuteAsync(Func> callback, TResult defaultValue, CancellationToken cancellationToken = default) + { + try + { + return await callback(cancellationToken).ConfigureAwait(false); + } + catch (RedisException) + { + return await callback(cancellationToken).ConfigureAwait(false); + } + } +} diff --git a/tests/UiPath.Caching.Tests/Redis/RedisCacheTests.cs b/tests/UiPath.Caching.Tests/Redis/RedisCacheTests.cs index 8ef5dac2..781a4035 100644 --- a/tests/UiPath.Caching.Tests/Redis/RedisCacheTests.cs +++ b/tests/UiPath.Caching.Tests/Redis/RedisCacheTests.cs @@ -5,6 +5,7 @@ using UiPath.Caching; using UiPath.Caching.Policies; using UiPath.Caching.Telemetry; +using UiPath.Caching.Tests.Fakes; using UiPath.Caching.Tests.Telemetry; namespace UiPath.Caching.Tests.Redis; @@ -391,6 +392,89 @@ public async Task GetCacheEntry_bundles_get_and_pttl_in_single_transaction() await _database.DidNotReceive().KeyExpireTimeAsync(_redisKey, Arg.Any()); } + [Fact] + public async Task GetCacheEntry_rebuilds_the_transaction_when_the_read_is_retried() + { + _resiliencePipelineProvider.Get(ResiliencePipelineNames.Read).Returns(new RetryOnceResiliencePipeline()); + var expected = _fixture.Create(); + var retried = Substitute.For(); + _database.CreateTransaction().Returns(_transaction, retried); + _transaction.StringGetAsync(_redisKey, CommandFlags.PreferReplica).Returns(Task.FromException(new RedisException("lost"))); + _transaction.KeyTimeToLiveAsync(_redisKey, CommandFlags.PreferReplica).Returns(Task.FromException(new RedisException("lost"))); + _transaction.ExecuteAsync(Arg.Any()).Returns(Task.FromException(new RedisException("timed out")), Task.FromResult(true)); + retried.StringGetAsync(_redisKey, CommandFlags.PreferReplica).Returns(_serializer.Serialize(expected)); + retried.KeyTimeToLiveAsync(_redisKey, CommandFlags.PreferReplica).Returns(TimeSpan.FromMinutes(15)); + retried.ExecuteAsync(Arg.Any()).Returns(true); + + var entry = await Sut.GetCacheEntryAsync(_cacheKey, policy: null, token: testContextAccessor.Current.CancellationToken); + + entry.Value.Should().Be(expected); + } + + [Fact] + public async Task Transactional_reads_run_the_read_pipeline_as_bool() + { + // One breaker per result type, so any other type would trip on its own. + var pipeline = new RecordingResiliencePipeline(); + _resiliencePipelineProvider.Get(ResiliencePipelineNames.Read).Returns(pipeline); + _transaction.ExecuteAsync(Arg.Any()).Returns(true); + + await Sut.GetCacheEntryAsync(_cacheKey, policy: null, token: testContextAccessor.Current.CancellationToken); + await Sut.GetCacheEntriesAsync(new CacheKey[] { _cacheKey, _multiKey }, policy: null, token: testContextAccessor.Current.CancellationToken); + + pipeline.ResultTypes.Should().OnlyContain(t => t == typeof(bool)).And.HaveCount(2); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task A_failed_transaction_leaves_no_unobserved_result(bool throws) + { + var marker = "unobserved-" + Guid.NewGuid().ToString("N"); + var unobserved = 0; + void OnUnobserved(object? sender, UnobservedTaskExceptionEventArgs e) + { + if (e.Exception.Flatten().InnerExceptions.Any(ex => ex.Message == marker)) + { + Interlocked.Increment(ref unobserved); + } + } + + TaskScheduler.UnobservedTaskException += OnUnobserved; + try + { + await ReadThroughAFailingTransactionAsync(marker, throws); + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + } + finally + { + TaskScheduler.UnobservedTaskException -= OnUnobserved; + } + + unobserved.Should().Be(0); + } + + [Fact] + public async Task GetCacheEntries_rebuilds_the_transaction_when_the_read_is_retried() + { + _resiliencePipelineProvider.Get(ResiliencePipelineNames.Read).Returns(new RetryOnceResiliencePipeline()); + var expected = _fixture.Create(); + var retried = Substitute.For(); + _database.CreateTransaction().Returns(_transaction, retried); + _transaction.StringGetAsync(Arg.Any(), CommandFlags.PreferReplica).Returns(Task.FromException(new RedisException("lost"))); + _transaction.KeyTimeToLiveAsync(Arg.Any(), CommandFlags.PreferReplica).Returns(Task.FromException(new RedisException("lost"))); + _transaction.ExecuteAsync(Arg.Any()).Returns(Task.FromException(new RedisException("timed out")), Task.FromResult(true)); + retried.StringGetAsync(Arg.Any(), CommandFlags.PreferReplica).Returns(new RedisValue[] { _serializer.Serialize(expected), RedisValue.Null }); + retried.KeyTimeToLiveAsync(Arg.Any(), CommandFlags.PreferReplica).Returns(TimeSpan.FromMinutes(15)); + retried.ExecuteAsync(Arg.Any()).Returns(true); + + var entries = await Sut.GetCacheEntriesAsync(new CacheKey[] { _cacheKey, _multiKey }, policy: null, token: testContextAccessor.Current.CancellationToken); + + entries[0].Value.Value.Should().Be(expected); + } + [Fact] public async Task GetCacheEntry_uses_KeyExpireTimeAsync_on_redis_v7() { @@ -1471,6 +1555,17 @@ public ValueTask InitializeAsync() return ValueTask.CompletedTask; } + [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.NoInlining)] + private async Task ReadThroughAFailingTransactionAsync(string marker, bool throws) + { + _version = new(6, 0); + _transaction.StringGetAsync(_redisKey, CommandFlags.PreferReplica).Returns(_ => Task.FromException(new RedisException(marker))); + _transaction.KeyTimeToLiveAsync(_redisKey, CommandFlags.PreferReplica).Returns(_ => Task.FromException(new RedisException(marker))); + _transaction.ExecuteAsync(Arg.Any()).Returns(_ => throws ? Task.FromException(new RedisException("transaction failed")) : Task.FromResult(false)); + + await Sut.GetCacheEntryAsync(_cacheKey, policy: null, token: testContextAccessor.Current.CancellationToken); + } + private void GiveKeysDifferentSlots() { _database.Multiplexer.GetHashSlot(_redisKey).Returns(1); diff --git a/tests/UiPath.Caching.Tests/Redis/RedisConnectionConfiguratorTests.cs b/tests/UiPath.Caching.Tests/Redis/RedisConnectionConfiguratorTests.cs index 6cf7f57c..e6816ff8 100644 --- a/tests/UiPath.Caching.Tests/Redis/RedisConnectionConfiguratorTests.cs +++ b/tests/UiPath.Caching.Tests/Redis/RedisConnectionConfiguratorTests.cs @@ -1,3 +1,4 @@ +using Microsoft.Extensions.Logging.Abstractions; using StackExchange.Redis; namespace UiPath.Caching.Tests.Redis; @@ -10,7 +11,7 @@ public async Task ApplyAsync_AppliesConfigurators() { var config = new ConfigurationOptions(); - await RedisConnectionConfigurators.ApplyAsync(config, [new ClientNameConfigurator("entra-test")], TestContext.Current.CancellationToken); + await RedisConnectionConfigurators.ApplyAsync(config, [new ClientNameConfigurator("entra-test")], Provider(), TestContext.Current.CancellationToken); config.ClientName.Should().Be("entra-test"); } @@ -20,7 +21,7 @@ public async Task ApplyAsync_AppliesConfigurators_InRegistrationOrder() { var config = new ConfigurationOptions(); - await RedisConnectionConfigurators.ApplyAsync(config, [new ClientNameConfigurator("first"), new ClientNameConfigurator("second")], TestContext.Current.CancellationToken); + await RedisConnectionConfigurators.ApplyAsync(config, [new ClientNameConfigurator("first"), new ClientNameConfigurator("second")], Provider(), TestContext.Current.CancellationToken); config.ClientName.Should().Be("second"); } @@ -30,7 +31,7 @@ public async Task ApplyAsync_WithNoConfigurators_LeavesBaseConfiguration() { var config = new ConfigurationOptions(); - await RedisConnectionConfigurators.ApplyAsync(config, [], TestContext.Current.CancellationToken); + await RedisConnectionConfigurators.ApplyAsync(config, [], Provider(), TestContext.Current.CancellationToken); config.ClientName.Should().BeNull(); } @@ -40,10 +41,106 @@ public async Task ApplyAsync_WithNullConfigurators_LeavesBaseConfiguration() { var config = new ConfigurationOptions(); - await RedisConnectionConfigurators.ApplyAsync(config, null, TestContext.Current.CancellationToken); + await RedisConnectionConfigurators.ApplyAsync(config, null, Provider(), TestContext.Current.CancellationToken); config.ClientName.Should().BeNull(); } + + [Fact] + public async Task ApplyAsync_KeepsARelaxedTimeoutAConfiguratorSet() + { + var optionsProvider = Provider("localhost:6379,asyncTimeout=1000"); + var config = optionsProvider.GetConfiguration(); + + await RedisConnectionConfigurators.ApplyAsync(config, [new RelaxedTimeoutConfigurator(TimeSpan.FromSeconds(42))], optionsProvider, TestContext.Current.CancellationToken); + + config.ToString().Should().Contain("maintRelaxedTimeout=42", "the configurator outranks the derived bound"); + } + + [Fact] + public async Task ApplyAsync_RederivesTheRelaxedTimeoutFromAnAsyncTimeoutAConfiguratorChanged() + { + var optionsProvider = Provider("localhost:6379,asyncTimeout=1000"); + var config = optionsProvider.GetConfiguration(); + + await RedisConnectionConfigurators.ApplyAsync(config, [new AsyncTimeoutConfigurator(30000)], optionsProvider, TestContext.Current.CancellationToken); + + config.ToString().Should().Contain("maintRelaxedTimeout=60", "the bound follows the timeout the connection ends up with"); + } + + [Fact] + public async Task ApplyAsync_LeavesTheRelaxedTimeoutUnsetWhenNoConfiguratorMovedTheAsyncTimeout() + { + // DI supplies an empty enumerable, not null. + var optionsProvider = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(new RedisConnectionOptions())); + var config = optionsProvider.GetConfiguration(); + + await RedisConnectionConfigurators.ApplyAsync(config, [], optionsProvider, TestContext.Current.CancellationToken); + + config.ToString().Should().NotContain("maintRelaxedTimeout"); + } + + [Fact] + public async Task ApplyAsync_DerivesTheRelaxedTimeoutWhenAConfiguratorSuppliesTheAsyncTimeout() + { + var optionsProvider = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(new RedisConnectionOptions())); + var config = optionsProvider.GetConfiguration(); + + await RedisConnectionConfigurators.ApplyAsync(config, [new AsyncTimeoutConfigurator(30000)], optionsProvider, TestContext.Current.CancellationToken); + + config.ToString().Should().Contain("maintRelaxedTimeout=60"); + } + + [Fact] + public async Task ApplyAsync_KeepsTheUpFrontDerivationWhenNoConfiguratorMovedTheAsyncTimeout() + { + var optionsProvider = Provider("localhost:6379,asyncTimeout=1000"); + var config = optionsProvider.GetConfiguration(); + + await RedisConnectionConfigurators.ApplyAsync(config, [new ClientNameConfigurator("noop")], optionsProvider, TestContext.Current.CancellationToken); + + config.ToString().Should().Contain("maintRelaxedTimeout=2"); + } + + [Fact] + public async Task ApplyAsync_KeepsARelaxedTimeoutAConfiguratorSetToTheValueItWouldHaveDerived() + { + // Assigns exactly what the derivation would give, so only presence can tell it was set. + var optionsProvider = Provider("localhost:6379,asyncTimeout=1000"); + var config = optionsProvider.GetConfiguration(); + + await RedisConnectionConfigurators.ApplyAsync( + config, + [new RelaxedTimeoutConfigurator(TimeSpan.FromSeconds(2)), new AsyncTimeoutConfigurator(30000)], + optionsProvider, + TestContext.Current.CancellationToken); + + config.ToString().Should().Contain("maintRelaxedTimeout=2", "the configurator assigned it, whatever the value"); + } + + private static RedisConfigurationOptionsProvider Provider(string connectionString = "localhost:6379") => + new(NullLoggerFactory.Instance, Options.Create(new RedisConnectionOptions { ConnectionString = connectionString })); + + private sealed class RelaxedTimeoutConfigurator(TimeSpan relaxed) : IRedisConnectionConfigurator + { + public ValueTask ConfigureAsync(ConfigurationOptions configuration, CancellationToken cancellationToken = default) + { +#pragma warning disable SER010 // Server-native maintenance notifications are for evaluation purposes only + configuration.MaintenanceRelaxedTimeout = relaxed; +#pragma warning restore SER010 + return ValueTask.CompletedTask; + } + } + + private sealed class AsyncTimeoutConfigurator(int asyncTimeout) : IRedisConnectionConfigurator + { + public ValueTask ConfigureAsync(ConfigurationOptions configuration, CancellationToken cancellationToken = default) + { + configuration.AsyncTimeout = asyncTimeout; + return ValueTask.CompletedTask; + } + } + private sealed class ClientNameConfigurator(string name) : IRedisConnectionConfigurator { public ValueTask ConfigureAsync(ConfigurationOptions configuration, CancellationToken cancellationToken = default) diff --git a/tests/UiPath.Caching.Tests/Redis/RedisConnectorTests.cs b/tests/UiPath.Caching.Tests/Redis/RedisConnectorTests.cs index 4cb94366..f0a737c3 100644 --- a/tests/UiPath.Caching.Tests/Redis/RedisConnectorTests.cs +++ b/tests/UiPath.Caching.Tests/Redis/RedisConnectorTests.cs @@ -59,6 +59,187 @@ public void ConnectionStringExtraParamsX(string connectionString, string extraPa cnn.Should().Be(expected); } + // On the rendered string: the option travels as whole seconds. + [Theory] + [InlineData("localhost:6379,asyncTimeout=3000", "maintRelaxedTimeout=6")] + [InlineData("localhost:6379,syncTimeout=3000", "maintRelaxedTimeout=6")] + // 500ms renders as zero without rounding up. + [InlineData("localhost:6379,asyncTimeout=250", "maintRelaxedTimeout=1")] + [InlineData("localhost:6379,asyncTimeout=400000", "maintRelaxedTimeout=600")] + public void MaintenanceRelaxedTimeout_IsDerived_FromTheAsyncTimeout(string connectionString, string expected) + { + var opt = new RedisConnectionOptions { ConnectionString = connectionString }; + var sut = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(opt)); + var config = sut.GetConfiguration(); + + sut.ReapplyDerivedBounds(config); + + config.ToString().Should().Contain(expected); + } + + [Fact] + public void MaintenanceRelaxedTimeout_TakesTheConfiguredValue_OverTheDerivedOne() + { + var opt = new RedisConnectionOptions + { + ConnectionString = "localhost:6379,asyncTimeout=3000", + MaintenanceRelaxedTimeout = TimeSpan.FromSeconds(42), + }; + var sut = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(opt)); + + sut.GetConfiguration().ToString().Should().Contain("maintRelaxedTimeout=42"); + } + + [Theory] + [InlineData(0.5, "maintRelaxedWindowMax=1")] + [InlineData(45d, "maintRelaxedWindowMax=45")] + [InlineData(900d, "maintRelaxedWindowMax=600")] + public void MaintenanceRelaxedWindowMax_IsAppliedInWholeSeconds(double configuredSeconds, string expected) + { + var opt = new RedisConnectionOptions + { + ConnectionString = "localhost:6379,asyncTimeout=3000", + MaintenanceRelaxedWindowMax = TimeSpan.FromSeconds(configuredSeconds), + }; + var sut = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(opt)); + + sut.GetConfiguration().ToString().Should().Contain(expected); + } + + [Fact] + public void MaintenanceRelaxedWindowMax_IsLeftToTheClient_WhenNotConfigured() + { + var opt = new RedisConnectionOptions { ConnectionString = "localhost:6379,asyncTimeout=3000" }; + var sut = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(opt)); + + sut.GetConfiguration().ToString().Should().NotContain("maintRelaxedWindowMax"); + } + + [Fact] + public void MaintenanceRelaxedTimeout_RoundsUpAConfiguredSubSecondValue() + { + var opt = new RedisConnectionOptions + { + ConnectionString = "localhost:6379,asyncTimeout=3000", + MaintenanceRelaxedTimeout = TimeSpan.FromMilliseconds(500), + }; + var sut = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(opt)); + + sut.GetConfiguration().ToString().Should().Contain("maintRelaxedTimeout=1"); + } + + [Fact] + public void MaintenanceRelaxedTimeout_IsNotDerived_WithoutAConnectionString() + { + var opt = new RedisConnectionOptions { ConnectionString = string.Empty }; + var sut = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(opt)); + + sut.GetConfiguration().ToString().Should().NotContain("maintRelaxedTimeout"); + } + + [Fact] + public void MaintenanceRelaxedTimeout_IsStillApplied_WithoutAConnectionString_WhenConfigured() + { + var opt = new RedisConnectionOptions + { + ConnectionString = string.Empty, + MaintenanceRelaxedTimeout = TimeSpan.FromSeconds(42), + }; + var sut = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(opt)); + + sut.GetConfiguration().ToString().Should().Contain("maintRelaxedTimeout=42"); + } + + [Theory] + [InlineData(null, true)] + [InlineData(true, true)] + [InlineData(false, false)] + public void FailFastBacklogPolicy_DefaultsToFailingFast(bool? configured, bool expectFailFast) + { + // Assigned even when null, or the initializer would be tested instead. + var opt = new RedisConnectionOptions { ConnectionString = "localhost:6379", FailFastBacklogPolicy = configured }; + var sut = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(opt)); + + var policy = sut.GetConfiguration().BacklogPolicy; + + policy.Should().BeSameAs(expectFailFast ? BacklogPolicy.FailFast : BacklogPolicy.Default); + } + + [Fact] + public void AnExplicitMaintRelaxedTimeout_InTheConnectionString_IsNotDerivedOver() + { + var opt = new RedisConnectionOptions { ConnectionString = "localhost:6379,asyncTimeout=5000,maintRelaxedTimeout=42" }; + var sut = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(opt)); + + sut.GetConfiguration().ToString().Should().Contain("maintRelaxedTimeout=42"); + + // Equal to the client default, so only presence can tell it was set. + var sameAsDefault = new RedisConnectionOptions { ConnectionString = "localhost:6379,asyncTimeout=1000,maintRelaxedTimeout=10" }; + var provider = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(sameAsDefault)); + + provider.GetConfiguration().ToString().Should().Contain("maintRelaxedTimeout=10"); + } + + [Fact] + public void TheRelaxedTimeout_IsDerivedFromTheAsyncTimeout_AConfiguratorLeftBehind() + { + var opt = new RedisConnectionOptions { ConnectionString = "localhost:6379,asyncTimeout=1000" }; + var sut = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(opt)); + var config = sut.GetConfiguration(); + + config.AsyncTimeout = 30000; + sut.ReapplyDerivedBounds(config); + + config.ToString().Should().Contain("maintRelaxedTimeout=60", "the bound follows the timeout the connection ends up with"); + } + + [Fact] + public void ReapplyDerivedBounds_HandlesAConnectionBuiltWithoutAConnectionString() + { + var sut = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(new RedisConnectionOptions { ConnectionString = null! })); + var config = sut.GetConfiguration(); + + var act = () => sut.ReapplyDerivedBounds(config); + + act.Should().NotThrow(); + config.ToString().Should().NotContain("maintRelaxedTimeout", "there is no configured timeout to derive from"); + } + + [Fact] + public void ReapplyDerivedBounds_FollowsASyncTimeoutAConfiguratorSetWithoutAConnectionString() + { + var sut = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(new RedisConnectionOptions { ConnectionString = null! })); + var config = sut.GetConfiguration(); + + config.SyncTimeout = 4000; + sut.ReapplyDerivedBounds(config); + + config.ToString().Should().Contain("maintRelaxedTimeout=8"); + } + + [Fact] + public void ReapplyDerivedBounds_LeavesASuppliedRelaxedTimeoutAlone() + { + var opt = new RedisConnectionOptions { ConnectionString = "localhost:6379,asyncTimeout=1000,maintRelaxedTimeout=42" }; + var sut = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(opt)); + var config = sut.GetConfiguration(); + + config.AsyncTimeout = 30000; + sut.ReapplyDerivedBounds(config); + + config.ToString().Should().Contain("maintRelaxedTimeout=42", "it was supplied, not derived"); + } + + [Fact] + public void FailFastBacklogPolicy_ReachesAConnectionBuiltWithoutAConnectionString() + { + var sut = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(new RedisConnectionOptions())); + + var policy = sut.GetConfiguration().BacklogPolicy; + + policy.Should().BeSameAs(BacklogPolicy.FailFast); + } + #pragma warning disable SER010 // Server-native maintenance notifications are for evaluation purposes only [Theory] [InlineData(RedisMaintenanceNotifications.Auto, MaintenanceNotificationMode.Auto)] diff --git a/tests/UiPath.Caching.Tests/Redis/RedisHashCacheTests.cs b/tests/UiPath.Caching.Tests/Redis/RedisHashCacheTests.cs index cdbc701e..1a5ebe43 100644 --- a/tests/UiPath.Caching.Tests/Redis/RedisHashCacheTests.cs +++ b/tests/UiPath.Caching.Tests/Redis/RedisHashCacheTests.cs @@ -6,6 +6,7 @@ using UiPath.Caching; using UiPath.Caching.Policies; using UiPath.Caching.Telemetry; +using UiPath.Caching.Tests.Fakes; using UiPath.Caching.Tests.Telemetry; using JsonSerializer = UiPath.Caching.SystemJsonByteSerializerProxy; @@ -258,6 +259,27 @@ public async Task Get_cache_entry_with_metadata_v7() actual.Value.Should().BeEquivalentTo(expected); } + [Fact] + public async Task Get_cache_entry_rebuilds_the_transaction_when_the_read_is_retried() + { + _pipelineProvider.Get(ResiliencePipelineNames.Read).Returns(new RetryOnceResiliencePipeline()); + var expected = _fixture.Create>(); + var entries = expected.Select(kv => new HashEntry(kv.Key, JsonConvert.SerializeObject(kv.Value))).ToArray(); + var retried = Substitute.For(); + _database.CreateTransaction().Returns(_transaction, retried); + _database.KeyExistsAsync(_redisKey, CommandFlags.PreferReplica).Returns(true); + _transaction.HashGetAllAsync(_redisKey, CommandFlags.PreferReplica).Returns(Task.FromException(new RedisException("lost"))); + _transaction.KeyTimeToLiveAsync(_redisKey, CommandFlags.PreferReplica).Returns(Task.FromException(new RedisException("lost"))); + _transaction.ExecuteAsync(Arg.Any()).Returns(Task.FromException(new RedisException("timed out")), Task.FromResult(true)); + retried.HashGetAllAsync(_redisKey, CommandFlags.PreferReplica).Returns(entries); + retried.KeyTimeToLiveAsync(_redisKey, CommandFlags.PreferReplica).Returns(TimeSpan.FromMinutes(15)); + retried.ExecuteAsync(Arg.Any()).Returns(true); + + var actual = await Sut.GetCacheEntryAsync(_cacheKey, policy: null, token: testContextAccessor.Current.CancellationToken); + + actual.Value.Should().BeEquivalentTo(expected); + } + [Fact] public async Task Get_cache_entry_metadata_v6() { diff --git a/tests/UiPath.Caching.Tests/Redis/RedisPlannedMaintenanceRoutingTests.cs b/tests/UiPath.Caching.Tests/Redis/RedisPlannedMaintenanceRoutingTests.cs index 6d2a53ca..7f1786ff 100644 --- a/tests/UiPath.Caching.Tests/Redis/RedisPlannedMaintenanceRoutingTests.cs +++ b/tests/UiPath.Caching.Tests/Redis/RedisPlannedMaintenanceRoutingTests.cs @@ -15,22 +15,25 @@ namespace UiPath.Caching.Tests.Redis; public class RedisPlannedMaintenanceRoutingTests : IDisposable { private readonly List _started = []; - private readonly MovableClock _clock = new(); private readonly RecordingTelemetryProvider _telemetry = new(); private readonly IConnectionMultiplexer _multiplexer = Substitute.For(); private readonly IRedisConnector _connector = Substitute.For(); + private readonly AdvanceableClock _clock = new(DateTimeOffset.UtcNow); + private readonly TaskCompletionSource _connectGate = new(TaskCreationOptions.RunContinuationsAsynchronously); + private IConnectionMultiplexer? _commandConnection; [Fact] - public async Task An_announced_disruption_is_recorded_without_reconnecting() + public async Task An_announced_disruption_opens_the_window_without_reconnecting() { // The client hands the connection off itself, and probing force-reconnects on a failed write. var sut = await StartedAsync(); RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrating)); + sut.InProgress.Should().BeTrue(); _connector.DidNotReceive().ForceReconnect(); - sut.InProgress.Should().BeFalse(); + _telemetry.Events.Should().Contain(e => e.Name == "Redis.MaintenanceStarted"); } [Theory] @@ -151,6 +154,459 @@ public async Task A_push_frame_on_the_maintenance_connection_is_ignored() _telemetry.Events.Should().NotContain(e => e.Name == "Redis.Maintenance"); } + [Fact] + public async Task Only_the_matching_family_closes_its_window() + { + // No tail, so only the outstanding failover can hold it open. + var sut = await StartedAsync("localhost:6379,maintPostEventRelaxed=0"); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrating)); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailingOver)); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrated)); + + sut.InProgress.Should().BeTrue("the failover is still outstanding"); + _telemetry.Events.Should().NotContain(e => e.Name == "Redis.MaintenanceEnded"); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailedOver)); + + sut.InProgress.Should().BeFalse(); + _telemetry.Events.Should().Contain(e => e.Name == "Redis.MaintenanceEnded"); + } + + [Fact] + public async Task An_interval_whose_start_was_refused_announces_no_end() + { + var telemetry = new ThrowOnMaintenanceEventProvider { FailingEvent = "Redis.MaintenanceStarted" }; + var sut = await StartedAsync("localhost:6379,maintPostEventRelaxed=0", telemetry); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailingOver)); + sut.InProgress.Should().BeTrue("the window is open whether or not the start could be announced"); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailedOver)); + + sut.InProgress.Should().BeFalse(); + telemetry.Events.Should().NotContain("Redis.MaintenanceEnded", "no start was ever announced"); + } + + [Fact] + public async Task The_Azure_probe_route_suggests_no_timeout() + { + var sut = await StartedAsync(); + + RaiseMaintenance(AzureEvent()); + + sut.InProgress.Should().BeTrue("the probe loop is running"); + sut.SuggestedTimeout.Should().BeNull("the connection is not relaxing its own timeouts on this route"); + } + + [Fact] + public async Task An_announced_window_suggests_what_the_client_relaxes_to() + { + var sut = await StartedAsync("localhost:6379,maintPostEventRelaxed=0"); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailingOver)); + + sut.InProgress.Should().BeTrue(); + sut.SuggestedTimeout.Should().NotBeNull("this route is one the client relaxes its own timeouts for"); + } + + [Fact] + public async Task A_window_is_sized_by_the_connection_it_arrived_on() + { + // Raised while this service's own connect is still inside the configurator pass. + var sut = await StartedWithGatedConfiguratorAsync(TimeSpan.FromSeconds(30)); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailingOver), DeliveredBy("localhost:6379,asyncTimeout=500,maintRelaxedTimeout=1,maintPostEventRelaxed=0")); + + sut.InProgress.Should().BeTrue(); + sut.SuggestedTimeout.Should().Be(TimeSpan.FromSeconds(1)); + } + + [Fact] + public async Task A_connection_is_resized_by_its_own_bounds_after_another_was_adopted() + { + var sut = await StartedAsync(); + var first = DeliveredBy("localhost:6379,maintRelaxedTimeout=1,maintPostEventRelaxed=0"); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailingOver, sequenceId: 1), first); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailingOver, sequenceId: 2), DeliveredBy("localhost:6379,maintRelaxedTimeout=60,maintPostEventRelaxed=0")); + sut.SuggestedTimeout.Should().Be(TimeSpan.FromSeconds(60)); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailingOver, sequenceId: 3), first); + + sut.SuggestedTimeout.Should().Be(TimeSpan.FromSeconds(1)); + } + + [Fact] + public async Task Notices_from_two_connections_are_handled_one_at_a_time() + { + // The first notice is held inside its record while the second arrives. + var telemetry = new BlockingFirstRecordProvider(); + await StartedAsync(telemetry: telemetry); + var first = Task.Run(() => RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrating, sequenceId: 1), DeliveredBy("localhost:6379,maintRelaxedTimeout=1")), TestContext.Current.CancellationToken); + await telemetry.Blocked.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + + var second = Task.Run(() => RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailingOver, sequenceId: 2), DeliveredBy("localhost:6379,maintRelaxedTimeout=60")), TestContext.Current.CancellationToken); + await Task.Delay(300, TestContext.Current.CancellationToken); + + second.IsCompleted.Should().BeFalse("the second notice waits for the first to finish"); + telemetry.Release(); + await Task.WhenAll(first, second).WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + } + + [Fact] + public async Task The_maintenance_connection_does_not_resize_a_window_its_sender_sized() + { + var sut = await StartedWithGatedConfiguratorAsync(TimeSpan.FromSeconds(60)); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrating), DeliveredBy("localhost:6379,maintRelaxedTimeout=1,maintPostEventRelaxed=0")); + + _connectGate.TrySetResult(); + await WaitForConnectedAsync(sut); + + sut.SuggestedTimeout.Should().Be(TimeSpan.FromSeconds(1)); + } + + [Fact] + public async Task A_replay_from_a_retired_connection_does_not_resize_the_window() + { + var sut = await StartedAsync(); + var frame = PushEvent(MaintenanceNotificationType.Migrating, sequenceId: 7); + RaiseOnCommandConnection(frame, DeliveredBy("localhost:6379,maintRelaxedTimeout=60,maintPostEventRelaxed=0")); + _clock.Advance(TimeSpan.FromSeconds(5)); + + RaiseOnCommandConnection(frame, DeliveredBy("localhost:6379,maintRelaxedTimeout=1,maintPostEventRelaxed=0")); + + sut.InProgress.Should().BeTrue(); + sut.SuggestedTimeout.Should().Be(TimeSpan.FromSeconds(60)); + } + + [Fact] + public async Task A_window_is_not_cut_short_before_this_service_connects() + { + // Sized by the client default, it would lapse at 10s. + var sut = await StartedWithGatedConfiguratorAsync(TimeSpan.FromSeconds(60)); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrating), DeliveredBy("localhost:6379,maintRelaxedTimeout=60,maintPostEventRelaxed=0")); + _clock.Advance(TimeSpan.FromSeconds(15)); + _connectGate.TrySetResult(); + await WaitForConnectedAsync(sut); + + sut.InProgress.Should().BeTrue(); + _telemetry.Events.Count(e => e.Name == "Redis.MaintenanceStarted").Should().Be(1); + _telemetry.Events.Should().NotContain(e => e.Name == "Redis.MaintenanceEnded"); + } + + [Fact] + public async Task A_configuration_that_will_not_parse_keeps_the_bounds_and_the_notice() + { + var sut = await StartedAsync("localhost:6379,maintPostEventRelaxed=0"); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailingOver), DeliveredBy("localhost:6379,notAKnownKey=1")); + + sut.InProgress.Should().BeTrue(); + _telemetry.Events.Should().Contain(e => e.Name == "Redis.Maintenance"); + } + + [Fact] + public async Task A_probe_rejected_while_disconnected_leaves_the_client_to_reconnect() + { + _connector.IsConnected.Returns(false); + FailProbes(ConnectionFailureType.UnableToConnect); + await StartedAsync(); + + RaiseMaintenance(AzureEvent()); + await Task.Delay(TimeSpan.FromSeconds(2.5), TestContext.Current.CancellationToken); + + _connector.DidNotReceive().ForceReconnect(); + } + + [Fact] + public async Task A_disconnect_that_outlasts_the_hanging_time_still_forces_a_reconnect() + { + _connector.IsConnected.Returns(false); + FailProbes(ConnectionFailureType.UnableToConnect); + await StartedAsync(); + + RaiseMaintenance(AzureEvent()); + await Task.Delay(TimeSpan.FromSeconds(1.5), TestContext.Current.CancellationToken); + _clock.Advance(TimeSpan.FromSeconds(11)); + + await WaitForAsync(() => _connector.ReceivedCalls().Any(c => c.GetMethodInfo().Name == nameof(IRedisConnector.ForceReconnect))); + } + + [Fact] + public async Task A_probe_failing_while_connected_forces_a_reconnect_at_once() + { + _connector.IsConnected.Returns(true); + FailProbes(ConnectionFailureType.SocketFailure); + await StartedAsync(); + + RaiseMaintenance(AzureEvent()); + + await WaitForAsync(() => _connector.ReceivedCalls().Any(c => c.GetMethodInfo().Name == nameof(IRedisConnector.ForceReconnect))); + } + + [Fact] + public async Task A_frame_replayed_later_in_a_long_window_is_still_collapsed() + { + var sut = await StartedAsync("localhost:6379,maintRelaxedTimeout=60,maintRelaxedWindowMax=120,maintPostEventRelaxed=0"); + var starter = PushEvent(MaintenanceNotificationType.FailingOver, announced: TimeSpan.FromSeconds(100)); + + RaiseOnCommandConnection(starter); + _clock.Advance(TimeSpan.FromSeconds(45)); + RaiseOnCommandConnection(starter); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailedOver)); + + sut.InProgress.Should().BeFalse("the replay was collapsed, so one completion closes the one operation"); + } + + [Fact] + public async Task Every_live_completion_earns_a_tail_not_just_the_last() + { + // The first completion's tail outlasts the second operation. + var sut = await StartedAsync("localhost:6379,maintRelaxedTimeout=10,maintRelaxedWindowMax=200,maintPostEventRelaxed=60"); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailingOver, sequenceId: 1, announced: TimeSpan.FromSeconds(20))); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailingOver, sequenceId: 2, announced: TimeSpan.FromSeconds(30))); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailedOver, sequenceId: 3)); + _clock.Advance(TimeSpan.FromSeconds(40)); + + sut.InProgress.Should().BeTrue("the completed operation's 60s tail outlives the 30s one still running"); + } + + [Fact] + public async Task The_window_maximum_caps_a_relaxed_timeout_above_it() + { + var sut = await StartedAsync("localhost:6379,maintRelaxedTimeout=10,maintRelaxedWindowMax=1,maintPostEventRelaxed=0"); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrating, announced: TimeSpan.FromSeconds(2))); + sut.InProgress.Should().BeTrue(); + + _clock.Advance(TimeSpan.FromSeconds(2)); + + sut.InProgress.Should().BeFalse("1s is the documented cap, whatever the floor says"); + } + + [Fact] + public async Task A_refused_record_does_not_release_the_claim_that_opened_a_window() + { + var telemetry = new ThrowOnMaintenanceEventProvider { FailingEvent = "Redis.Maintenance" }; + var sut = await StartedAsync("localhost:6379,maintPostEventRelaxed=0", telemetry); + var moving = PushEvent(MaintenanceNotificationType.FailingOver); + + RaiseOnCommandConnection(moving); + RaiseOnCommandConnection(moving); + + sut.InProgress.Should().BeTrue("the window is open"); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailedOver)); + + sut.InProgress.Should().BeFalse("one completion closes the one operation the replay must not have doubled"); + } + + [Fact] + public async Task A_window_is_resized_when_its_connection_brings_longer_bounds() + { + var sut = await StartedAsync(); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrating), DeliveredBy("localhost:6379,maintRelaxedTimeout=10,maintPostEventRelaxed=0")); + _clock.Advance(TimeSpan.FromSeconds(5)); + + RaiseOnCommandConnection(UnmodelledEvent("longer bounds"), DeliveredBy("localhost:6379,maintRelaxedTimeout=60,maintPostEventRelaxed=0")); + _clock.Advance(TimeSpan.FromSeconds(30)); + + sut.InProgress.Should().BeTrue("60s is what the connection now relaxes to, so the open window runs to that"); + } + + [Fact] + public async Task Shortened_bounds_close_an_open_window_at_once_rather_than_at_the_old_deadline() + { + var sut = await StartedAsync(); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrating), DeliveredBy("localhost:6379,maintRelaxedTimeout=10,maintPostEventRelaxed=0")); + _clock.Advance(TimeSpan.FromSeconds(5)); + + RaiseOnCommandConnection(UnmodelledEvent("shorter bounds"), DeliveredBy("localhost:6379,maintRelaxedTimeout=1,maintPostEventRelaxed=0")); + + sut.InProgress.Should().BeFalse("1s is what the connection now relaxes to, and 5s have passed"); + await WaitForAsync(() => _telemetry.Events.Any(e => e.Name == "Redis.MaintenanceEnded")); + } + + [Fact] + public async Task Stopping_does_not_announce_an_interval_a_pending_probe_left_behind() + { + // Stopped before the queued probe worker runs, with the flag set and nothing announced. + var sut = await StartedAsync(); + sut.InProgress = true; + + await sut.StopAsync(TestContext.Current.CancellationToken); + + _telemetry.Events.Should().NotContain(e => e.Name == "Redis.MaintenanceStarted"); + _telemetry.Events.Should().NotContain(e => e.Name == "Redis.MaintenanceEnded"); + } + + [Fact] + public async Task A_wall_clock_correction_forward_does_not_close_an_open_window() + { + var sut = await StartedAsync(); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.SlotMigrating, announced: TimeSpan.FromSeconds(30))); + + _clock.CorrectWallClock(TimeSpan.FromMinutes(10)); + + sut.InProgress.Should().BeTrue("no time has actually elapsed, whatever the wall clock now reads"); + } + + [Fact] + public async Task A_wall_clock_correction_backward_does_not_hold_a_window_past_its_duration() + { + var sut = await StartedAsync("localhost:6379,maintPostEventRelaxed=0"); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.SlotMigrating, announced: TimeSpan.FromSeconds(30))); + + _clock.CorrectWallClock(TimeSpan.FromMinutes(-10)); + _clock.Advance(TimeSpan.FromSeconds(31)); + + sut.InProgress.Should().BeFalse("30 seconds elapsed however far back the wall clock was set"); + } + + [Fact] + public async Task A_shorter_overlapping_operation_does_not_shorten_a_longer_one() + { + var sut = await StartedAsync("localhost:6379,maintPostEventRelaxed=0"); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.SlotMigrating, sequenceId: 16, announced: TimeSpan.FromSeconds(30))); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.SlotMigrating, sequenceId: 18, announced: TimeSpan.FromSeconds(5))); + + _clock.Advance(TimeSpan.FromSeconds(15)); + + sut.InProgress.Should().BeTrue("the 30s migration is still running, whatever the later one announced"); + } + + [Fact] + public async Task A_late_completion_does_not_release_a_longer_operation_still_running() + { + var sut = await StartedAsync("localhost:6379,maintPostEventRelaxed=0,maintRelaxedTimeout=5,maintRelaxedWindowMax=120"); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.SlotMigrating, sequenceId: 16, announced: TimeSpan.FromSeconds(60))); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.SlotMigrating, sequenceId: 18, announced: TimeSpan.FromSeconds(2))); + _clock.Advance(TimeSpan.FromSeconds(10)); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailedOver, sequenceId: 20)); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.SlotMigrated, sequenceId: 19)); + + sut.InProgress.Should().BeTrue("the completion may be the lapsed operation's, and the 60s one is still running"); + } + + [Fact] + public async Task A_replay_after_its_claim_aged_out_does_not_add_to_a_family_still_running() + { + // Starters at 0s and 30s keep the family alive past the first claim at 60s. + var sut = await StartedAsync("localhost:6379,maintPostEventRelaxed=0,maintRelaxedTimeout=5,maintRelaxedWindowMax=60"); + var first = PushEvent(MaintenanceNotificationType.Migrating, sequenceId: 1, announced: TimeSpan.FromSeconds(60)); + RaiseOnCommandConnection(first); + _clock.Advance(TimeSpan.FromSeconds(30)); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrating, sequenceId: 2, announced: TimeSpan.FromSeconds(60))); + _clock.Advance(TimeSpan.FromSeconds(31)); + + RaiseOnCommandConnection(first); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrated, sequenceId: 3)); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrated, sequenceId: 4)); + + sut.InProgress.Should().BeFalse("two operations ran and two completed; the replay added none"); + } + + [Fact] + public async Task Overlapping_operations_of_one_family_each_hold_the_window() + { + // A completion has its own sequence id, so nothing pairs it with a starter. + var sut = await StartedAsync("localhost:6379,maintPostEventRelaxed=0"); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.SlotMigrating, sequenceId: 16)); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.SlotMigrating, sequenceId: 18)); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.SlotMigrated, sequenceId: 17)); + + sut.InProgress.Should().BeTrue("the second migration is still running"); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.SlotMigrated, sequenceId: 19)); + + sut.InProgress.Should().BeFalse("both have completed"); + } + + [Fact] + public async Task A_completion_hands_over_to_the_tail_the_client_stays_relaxed_for() + { + var sut = await StartedAsync(); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrating)); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrated)); + + sut.InProgress.Should().BeTrue("the client is still relaxed for maintPostEventRelaxed"); + + _clock.Advance(TimeSpan.FromSeconds(21)); + + sut.InProgress.Should().BeFalse(); + } + + [Fact] + public async Task A_completion_arriving_after_its_window_lapsed_does_not_reopen_it() + { + var sut = await StartedAsync(); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrating, announced: TimeSpan.FromSeconds(15))); + + // Without firing the timer, so the lapsed entry is still present. + _clock.AdvanceWithoutFiringTimers(TimeSpan.FromSeconds(16)); + sut.InProgress.Should().BeFalse(); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrated)); + + sut.InProgress.Should().BeFalse("a lapsed window is over, and its completion cannot restart it"); + _telemetry.Events.Count(e => e.Name == "Redis.MaintenanceStarted").Should().Be(1, "one disruption is one interval"); + } + + [Fact] + public async Task A_new_operation_does_not_discard_the_tail_an_earlier_one_earned() + { + var sut = await StartedAsync(); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrating)); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrated)); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrating)); + _clock.Advance(TimeSpan.FromSeconds(15)); + + sut.InProgress.Should().BeTrue("the 20s tail outlives the 10s window the new operation opened"); + } + + [Fact] + public async Task A_completion_closes_at_once_when_there_is_no_tail() + { + var sut = await StartedAsync("localhost:6379,maintPostEventRelaxed=0"); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailingOver)); + + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.FailedOver)); + + sut.InProgress.Should().BeFalse("nothing is left relaxed, so nothing is left to suppress"); + } + + [Fact] + public async Task A_window_whose_completion_never_arrives_lapses() + { + var sut = await StartedAsync(); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Moving, announced: TimeSpan.FromSeconds(15))); + sut.InProgress.Should().BeTrue(); + + _clock.Advance(TimeSpan.FromSeconds(16)); + + sut.InProgress.Should().BeFalse(); + await WaitForAsync(() => _telemetry.Events.Any(e => e.Name == "Redis.MaintenanceEnded")); + } + + [Fact] + public async Task An_announced_duration_shorter_than_a_reconnect_is_widened() + { + var sut = await StartedAsync(); + RaiseOnCommandConnection(PushEvent(MaintenanceNotificationType.Migrating, announced: TimeSpan.FromSeconds(2))); + + _clock.Advance(TimeSpan.FromSeconds(5)); + + sut.InProgress.Should().BeTrue(); + } + [Fact] public async Task Every_push_notification_is_recorded_with_its_source() { @@ -303,7 +759,7 @@ public async Task A_throw_while_handling_a_notification_does_not_escape_the_main { // Attached straight to the client, unlike the command route, so nothing else keeps throws off its dispatch. var telemetry = new ThrowOnMaintenanceEventProvider(); - await StartedAsync(telemetry); + await StartedAsync(telemetry: telemetry); var raise = () => RaiseMaintenance(AzureEvent()); @@ -316,7 +772,7 @@ public async Task A_report_that_fails_too_does_not_escape_the_maintenance_connec { // A sink that cannot take the report leaves nowhere to put it, and it still must not reach the dispatch thread. var telemetry = new ThrowOnMaintenanceEventProvider { ReportingThrows = true }; - await StartedAsync(telemetry); + await StartedAsync(telemetry: telemetry); var raise = () => RaiseMaintenance(AzureEvent()); @@ -324,18 +780,17 @@ public async Task A_report_that_fails_too_does_not_escape_the_maintenance_connec } [Fact] - public async Task A_notification_whose_recording_threw_is_recorded_when_it_arrives_again() + public async Task A_refused_record_is_reported_and_not_retried() { - // A handler that threw recorded nothing, so holding its claim would lose the notification for the window. var telemetry = new ThrowOnMaintenanceEventProvider(failures: 1); - await StartedAsync(telemetry); + await StartedAsync(telemetry: telemetry); var azure = AzureEvent(); RaiseMaintenance(azure); RaiseMaintenance(azure); - telemetry.Exceptions.Should().Contain(ThrowOnMaintenanceEventProvider.Failure); - telemetry.Events.Count(e => e == "Redis.Maintenance").Should().Be(1, "the retry recorded what the failed attempt did not"); + telemetry.Exceptions.Should().Contain(ThrowOnMaintenanceEventProvider.Failure, "the refusal is reported"); + telemetry.Events.Should().NotContain("Redis.Maintenance", "the copy is collapsed, so nothing re-records it"); } [Fact] @@ -344,7 +799,7 @@ public async Task Disposing_reaches_the_multiplexer_when_cancelling_and_reportin // Cancel runs the probe loop's registrations, so it can throw what a caller did; the rest of Dispose // still has a subscribed connection to let go of. var telemetry = new ThrowOnMaintenanceEventProvider { ReportingThrows = true }; - var sut = await StartedAsync(telemetry); + var sut = await StartedAsync(telemetry: telemetry); var source = (CancellationTokenSource)typeof(RedisPlannedMaintenance) .GetField("_cancellationTokenSource", BindingFlags.Instance | BindingFlags.NonPublic)! .GetValue(sut)!; @@ -362,7 +817,7 @@ public async Task A_probe_run_whose_start_could_not_be_announced_keeps_probing() // Announcing is not what the run is for: losing the event must not cost the probing and the ForceReconnect // that are the Azure route's whole recovery. var telemetry = new ThrowOnMaintenanceEventProvider { FailingEvent = "Redis.MaintenanceStarted" }; - var sut = await StartedAsync(telemetry); + var sut = await StartedAsync(telemetry: telemetry); RaiseMaintenance(AzureEvent()); await WaitForRefusalAsync(telemetry); @@ -378,7 +833,7 @@ public async Task A_probe_run_whose_start_could_not_be_announced_keeps_probing() public async Task A_probe_run_whose_end_could_not_be_announced_reports_that_rather_than_faulting() { var telemetry = new ThrowOnMaintenanceEventProvider { FailingEvent = "Redis.MaintenanceEnded" }; - var sut = await StartedAsync(telemetry); + var sut = await StartedAsync(telemetry: telemetry); RaiseMaintenance(AzureEvent()); for (var i = 0; i < 500 && !telemetry.Events.Contains("Redis.MaintenanceStarted"); i++) @@ -485,6 +940,28 @@ private static AzureMaintenanceEvent AzureEvent(string startTime = "2026-09-19T0 null, [$"NotificationType|NodeMaintenanceStarting|StartTimeInUTC|{startTime}|IsReplica|False|IPAddress|127.0.0.1|SSLPort|6380|NonSSLPort|6379"], null)!; + private static async Task WaitForConnectedAsync(RedisPlannedMaintenance sut) + { + // Assigned once the connect has finished. + var field = typeof(RedisPlannedMaintenance).GetField("_multiplexer", BindingFlags.Instance | BindingFlags.NonPublic)!; + for (var i = 0; i < 200 && field.GetValue(sut) is null; i++) + { + await Task.Delay(10, TestContext.Current.CancellationToken); + } + + field.GetValue(sut).Should().NotBeNull(); + } + + private static async Task WaitForAsync(Func condition) + { + // The delay continuation runs off the advancing thread. + for (var i = 0; i < 200 && !condition(); i++) + { + await Task.Delay(10, TestContext.Current.CancellationToken); + } + + condition().Should().BeTrue("the lapsed window must record its end, not just stop reporting"); + } /// A connection attempt that never succeeds, but that ends when the service is disposed. private static Task NeverConnects(CancellationToken cancellationToken) @@ -505,6 +982,14 @@ private static ServerMaintenanceEvent UnmodelledEvent(string rawMessage) return unmodelled; } + /// A command connection built with , as the connector reports it. + private static IConnectionMultiplexer DeliveredBy(string configuration) + { + var multiplexer = Substitute.For(); + multiplexer.Configuration.Returns(configuration); + return multiplexer; + } + /// Null is a frame whose sequence could not be read: zero, described as unknown. private static PushMaintenanceEvent PushEvent(MaintenanceNotificationType type, long? sequenceId = 1, TimeSpan? announced = null) => (PushMaintenanceEvent)Activator.CreateInstance( @@ -525,7 +1010,8 @@ private static PushMaintenanceEvent PushEvent(MaintenanceNotificationType type, private static async Task WaitForRefusalAsync(ThrowOnMaintenanceEventProvider telemetry) { - for (var i = 0; i < 500 && telemetry.RefusedAttempts == 0; i++) + // The count rises inside the throw, before the catch has reported it. + for (var i = 0; i < 500 && (telemetry.RefusedAttempts == 0 || telemetry.Exceptions.Length == 0); i++) { await Task.Delay(10, TestContext.Current.CancellationToken); } @@ -538,18 +1024,43 @@ private void RaiseMaintenance(ServerMaintenanceEvent e) => _multiplexer.ServerMaintenanceEvent += Raise.Event>(_multiplexer, e); /// As the connection carrying commands sees it — the route push frames are taken from. - private void RaiseOnCommandConnection(ServerMaintenanceEvent e) => - _connector.ServerMaintenance += Raise.Event>(_connector, e); + private void RaiseOnCommandConnection(ServerMaintenanceEvent e, object? sender = null) => + _connector.ServerMaintenance += Raise.Event>(sender ?? _commandConnection ?? (object)_connector, e); + + /// The command connection the connector would build from . + private void UseCommandConnection(string connectionString) + { + var provider = new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, Options.Create(new RedisConnectionOptions { ConnectionString = connectionString })); + var configuration = provider.GetConfiguration(); + provider.ReapplyDerivedBounds(configuration); + _commandConnection = DeliveredBy(configuration.ToString()); + } + + /// Fails every probe write, whichever overload the probe binds to. + private void FailProbes(ConnectionFailureType failure) + { + Task Fail() => Task.FromException(new RedisConnectionException(failure, CommandFlags.None, "probe")); + _connector.Database.StringSetAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()).Returns(_ => Fail()); + _connector.Database.StringSetAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()).Returns(_ => Fail()); + _connector.Database.StringSetAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()).Returns(_ => Fail()); + _connector.Database.StringSetAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()).Returns(_ => Fail()); + } private async Task StartedWithoutMaintenanceConnectionAsync() { + UseCommandConnection("localhost:6379"); var options = Options.Create(new RedisConnectionOptions { ConnectionString = "localhost:6379" }); var factory = Substitute.For(); // CA2012: arranging a ValueTask-returning member with NSubstitute means calling it and handing the // result to Returns. There is no shape of this that awaits it, and nothing ever consumes it. #pragma warning disable CA2012 + var creating = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); factory.CreateAsync(Arg.Any(), Arg.Any()) - .Returns(call => new ValueTask(NeverConnects(call.Arg()))); + .Returns(call => + { + creating.TrySetResult(); + return new ValueTask(NeverConnects(call.Arg())); + }); #pragma warning restore CA2012 var sut = new RedisPlannedMaintenance( @@ -564,12 +1075,17 @@ private async Task StartedWithoutMaintenanceConnectionA _started.Add(sut); await sut.StartAsync(TestContext.Current.CancellationToken); + + // Created once the connect has passed the configurators. + await creating.Task.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); return sut; } - private async Task StartedAsync(ICachingTelemetryProvider? telemetry = null) + private async Task StartedWithGatedConfiguratorAsync(TimeSpan relaxedTimeout, string connectionString = "localhost:6379", ICachingTelemetryProvider? telemetry = null) { - var options = Options.Create(new RedisConnectionOptions { ConnectionString = "localhost:6379" }); + UseCommandConnection(connectionString); + var configurator = new RelaxedTimeoutConfigurator(relaxedTimeout, _connectGate.Task); + var options = Options.Create(new RedisConnectionOptions { ConnectionString = connectionString }); var factory = Substitute.For(); // CA2012: arranging a ValueTask-returning member with NSubstitute means calling it and handing the // result to Returns. There is no shape of this that awaits it, and nothing ever consumes it. @@ -578,6 +1094,33 @@ private async Task StartedAsync(ICachingTelemetryProvid .Returns(_ => new ValueTask(_multiplexer)); #pragma warning restore CA2012 + var sut = new RedisPlannedMaintenance( + telemetry ?? _telemetry, + _connector, + new RedisConfigurationOptionsProvider(NullLoggerFactory.Instance, options), + factory, + NullLogger.Instance, + options, + [configurator], + _clock); + + _started.Add(sut); + await sut.StartAsync(TestContext.Current.CancellationToken); + return sut; + } + + private async Task StartedAsync(string connectionString = "localhost:6379", ICachingTelemetryProvider? telemetry = null) + { + UseCommandConnection(connectionString); + var options = Options.Create(new RedisConnectionOptions { ConnectionString = connectionString }); + var factory = Substitute.For(); + // CA2012: arranging a ValueTask-returning member with NSubstitute means calling it and handing the + // result to Returns. There is no shape of this that awaits it, and nothing ever consumes it. +#pragma warning disable CA2012 + factory.CreateAsync(Arg.Any(), Arg.Any()) + .Returns(_ => new ValueTask(_connectGate.Task.ContinueWith(_ => _multiplexer, TaskScheduler.Default))); +#pragma warning restore CA2012 + var sut = new RedisPlannedMaintenance( telemetry ?? _telemetry, _connector, @@ -590,6 +1133,7 @@ private async Task StartedAsync(ICachingTelemetryProvid _started.Add(sut); await sut.StartAsync(TestContext.Current.CancellationToken); + _connectGate.TrySetResult(); // StartAsync subscribes on a background task. The multiplexer field is assigned after the handler is // attached, so seeing it set means an event raised now will be delivered. @@ -604,6 +1148,31 @@ private async Task StartedAsync(ICachingTelemetryProvid } /// Fails the recording step, the way a telemetry sink under load would. + /// Holds the first Redis.Maintenance record until released. + private sealed class BlockingFirstRecordProvider : ICachingTelemetryProvider + { + private readonly TaskCompletionSource _blocked = new(TaskCreationOptions.RunContinuationsAsynchronously); + private readonly ManualResetEventSlim _release = new(); + private int _records; + + public Task Blocked => _blocked.Task; + + public void Release() => _release.Set(); + + public void TrackEvent(string eventName, ReadOnlySpan> properties = default, ReadOnlySpan> metrics = default) + { + if (eventName == "Redis.Maintenance" && Interlocked.Increment(ref _records) == 1) + { + _blocked.TrySetResult(); + _release.Wait(TimeSpan.FromSeconds(10)); + } + } + + public void TrackException(Exception ex, ReadOnlySpan> properties = default, ReadOnlySpan> metrics = default) + { + } + } + private sealed class ThrowOnMaintenanceEventProvider(int failures = int.MaxValue) : ICachingTelemetryProvider { public static readonly InvalidOperationException Failure = new("telemetry boom"); @@ -622,9 +1191,9 @@ private sealed class ThrowOnMaintenanceEventProvider(int failures = int.MaxValue /// Counts the attempts that were refused, so a test can wait for one without racing it. public int RefusedAttempts => Volatile.Read(ref _refused); - public IReadOnlyList Exceptions => Snapshot(_exceptions); + public Exception[] Exceptions => Snapshot(_exceptions); - public IReadOnlyList Events => Snapshot(_events); + public string[] Events => Snapshot(_events); public void TrackEvent(string eventName, ReadOnlySpan> properties = default, ReadOnlySpan> metrics = default) { @@ -662,24 +1231,128 @@ private T[] Snapshot(List source) } } - /// A clock whose timestamps move only when a test moves them, and whose wall reading can disagree. - private sealed class MovableClock : TimeProvider +} + +#pragma warning restore SER010 + +/// A clock a test moves; its timers fire on an advance. +internal sealed class AdvanceableClock(DateTimeOffset now) : TimeProvider +{ + private readonly List _timers = []; + private DateTimeOffset _now = now; + private TimeSpan _wallOffset; + + public override long TimestampFrequency => TimeSpan.TicksPerSecond; + + public override DateTimeOffset GetUtcNow() + { + lock (_timers) + { + return _now + _wallOffset; + } + } + + public override long GetTimestamp() + { + lock (_timers) + { + return _now.UtcTicks; + } + } + + /// Moves the wall clock only. + public void CorrectWallClock(TimeSpan by) { - private long _timestamp = 1_000_000; - private TimeSpan _wallOffset; + lock (_timers) + { + _wallOffset += by; + } + } - public override long TimestampFrequency => TimeSpan.TicksPerSecond; + public override ITimer CreateTimer(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period) + { + var timer = new FakeTimer(this, callback, state); + timer.Schedule(dueTime); + lock (_timers) + { + _timers.Add(timer); + } - public override DateTimeOffset GetUtcNow() => - new DateTimeOffset(2026, 9, 19, 0, 0, 0, TimeSpan.Zero).AddTicks(_timestamp) + _wallOffset; + return timer; + } - public override long GetTimestamp() => _timestamp; + public void Advance(TimeSpan by) + { + FakeTimer[] due; + lock (_timers) + { + _now = _now.Add(by); + due = _timers.Where(timer => timer.IsDue(_now.UtcTicks)).ToArray(); + } - public void Advance(TimeSpan by) => _timestamp += by.Ticks; + foreach (var timer in due) + { + timer.Fire(); + } + } - /// What an NTP correction does to the wall clock and not to the timestamps. - public void CorrectWallClock(TimeSpan by) => _wallOffset += by; + /// Moves time without running the timers now due. + public void AdvanceWithoutFiringTimers(TimeSpan by) + { + lock (_timers) + { + _now = _now.Add(by); + } + } + + internal void Forget(FakeTimer timer) + { + lock (_timers) + { + _timers.Remove(timer); + } + } + + internal sealed class FakeTimer(AdvanceableClock clock, TimerCallback callback, object? state) : ITimer + { + private long? _dueTicks; + + public bool Change(TimeSpan dueTime, TimeSpan period) + { + Schedule(dueTime); + return true; + } + + public void Dispose() => clock.Forget(this); + + public ValueTask DisposeAsync() + { + Dispose(); + return ValueTask.CompletedTask; + } + + // Monotonic, so a wall-clock correction cannot move an armed timer. + internal void Schedule(TimeSpan dueTime) => + _dueTicks = dueTime == Timeout.InfiniteTimeSpan ? null : clock.GetTimestamp() + dueTime.Ticks; + + internal bool IsDue(long nowTicks) => _dueTicks is { } due && due <= nowTicks; + + internal void Fire() + { + _dueTicks = null; + callback(state); + } } } +/// Sets the relaxed timeout once the gate opens. +internal sealed class RelaxedTimeoutConfigurator(TimeSpan relaxedTimeout, Task gate) : IRedisConnectionConfigurator +{ + public async ValueTask ConfigureAsync(ConfigurationOptions configuration, CancellationToken cancellationToken = default) + { + await gate.ConfigureAwait(false); +#pragma warning disable SER010 // Server-native maintenance notifications are for evaluation purposes only + configuration.MaintenanceRelaxedTimeout = relaxedTimeout; #pragma warning restore SER010 + } +} diff --git a/tests/UiPath.Caching.Tests/Redis/RedisSetCacheTests.cs b/tests/UiPath.Caching.Tests/Redis/RedisSetCacheTests.cs index d3f949a5..24d70971 100644 --- a/tests/UiPath.Caching.Tests/Redis/RedisSetCacheTests.cs +++ b/tests/UiPath.Caching.Tests/Redis/RedisSetCacheTests.cs @@ -35,6 +35,17 @@ public class RedisSetCacheTests(ITestContextAccessor testContextAccessor) : IAsy [Fact] public void Name_is_Redis() => Sut.Name.Should().Be("Redis"); + [Fact] + public void The_read_pipeline_is_refused_for_destructive_reads() + { + _setCacheOptions.ResilienceKeyName = ResiliencePipelineNames.Read; + + var act = () => Sut; + + act.Should().Throw().Which.GetBaseException().Should().BeOfType() + .Which.Message.Should().Contain(nameof(RedisSetCacheOptions.ResilienceKeyName)); + } + [Fact] public void Uses_se_key_prefix() { @@ -94,6 +105,37 @@ public async Task Add_single_redis_exception_returns_false() _logger.ReceivedCalls().Should().Contain(c => c.GetMethodInfo().Name == "Log" && (LogLevel)c.GetArguments()[0]! == LogLevel.Warning); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task A_failed_add_leaves_no_unobserved_result(bool throws) + { + var marker = "unobserved-" + Guid.NewGuid().ToString("N"); + var unobserved = 0; + void OnUnobserved(object? sender, UnobservedTaskExceptionEventArgs e) + { + if (e.Exception.Flatten().InnerExceptions.Any(ex => ex.Message == marker)) + { + Interlocked.Increment(ref unobserved); + } + } + + TaskScheduler.UnobservedTaskException += OnUnobserved; + try + { + await AddThroughAFailingTransactionAsync(marker, throws); + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + } + finally + { + TaskScheduler.UnobservedTaskException -= OnUnobserved; + } + + unobserved.Should().Be(0); + } + [Fact] public async Task Add_many_works() { @@ -565,6 +607,15 @@ public ValueTask InitializeAsync() return ValueTask.CompletedTask; } + [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.NoInlining)] + private async Task AddThroughAFailingTransactionAsync(string marker, bool throws) + { + _transaction.SetAddAsync(_redisKey, Arg.Any(), CommandFlags.DemandMaster).Returns(_ => Task.FromException(new RedisException(marker))); + _transaction.ExecuteAsync(Arg.Any()).Returns(_ => throws ? Task.FromException(new RedisException("transaction failed")) : Task.FromResult(false)); + + await Sut.AddAsync(_cacheKey, _fixture.Create(), policy: null, token: testContextAccessor.Current.CancellationToken); + } + private sealed class CountingPipeline : IResiliencePipeline { public int Calls { get; private set; } diff --git a/tests/UiPath.Caching.Tests/ResiliencePipelineWrapperTests.cs b/tests/UiPath.Caching.Tests/ResiliencePipelineWrapperTests.cs index 03fa7fe5..4fe51a0c 100644 --- a/tests/UiPath.Caching.Tests/ResiliencePipelineWrapperTests.cs +++ b/tests/UiPath.Caching.Tests/ResiliencePipelineWrapperTests.cs @@ -1,9 +1,16 @@ +using System.Diagnostics; +using Microsoft.Extensions.Logging.Abstractions; using Polly; +using Polly.Timeout; +using UiPath.Caching.Policies; using UiPath.Caching.Polly; namespace UiPath.Caching.Tests; public class ResiliencePipelineWrapperTests(ITestContextAccessor testContextAccessor) : IAsyncLifetime { + private static readonly TimeSpan PipelineTimeout = TimeSpan.FromMilliseconds(100); + private static readonly TimeSpan CallbackDuration = TimeSpan.FromSeconds(1); + private readonly IFixture _fixture = AutoFixtureCreator.NSubstitute(); private IResiliencePipelineFactory _resiliencePipelineFactory = default!; private int _boolCallCount = 0; @@ -48,6 +55,120 @@ public async Task AllCached() _intCallCount.Should().Be(1); } + [Fact] + public async Task The_read_pipeline_abandons_a_callback_that_ignores_the_token() + { + var sut = CreateTimedSut(ResiliencePipelineNames.Read); + var elapsed = Stopwatch.StartNew(); + + var act = async () => await sut.ExecuteAsync(IgnoresTheToken, false, testContextAccessor.Current.CancellationToken); + + await act.Should().ThrowAsync(); + elapsed.Elapsed.Should().BeLessThan(CallbackDuration); + } + + [Theory] + [InlineData(ResiliencePipelineNames.Write)] + [InlineData("set-pop")] + public async Task Other_pipelines_await_a_callback_that_ignores_the_token(string scope) + { + var sut = CreateTimedSut(scope); + + var result = await sut.ExecuteAsync(IgnoresTheToken, false, testContextAccessor.Current.CancellationToken); + + result.Should().BeTrue("the pipeline returned only once the callback had finished"); + } + + [Fact] + public async Task A_callback_that_finishes_inside_the_timeout_is_unaffected() + { + var sut = CreateTimedSut(ResiliencePipelineNames.Read); + + var result = await sut.ExecuteAsync(_ => new ValueTask(true), false, testContextAccessor.Current.CancellationToken); + + result.Should().BeTrue(); + } + + [Fact] + public async Task An_announced_window_widens_the_timeout() + { + var sut = CreateTimedSut(ResiliencePipelineNames.Read, disruptionTimeout: TimeSpan.FromSeconds(3), disruptionInProgress: true, suggestedTimeout: TimeSpan.FromMilliseconds(120)); + + var result = await sut.ExecuteAsync(IgnoresTheToken, false, testContextAccessor.Current.CancellationToken); + + result.Should().BeTrue("the call outlived the configured timeout but not the one the disruption allows"); + } + + [Fact] + public async Task A_disruption_with_no_announced_window_does_not_widen_even_when_configured() + { + // The Azure probe route: in progress, with no announced window. + var sut = CreateTimedSut(ResiliencePipelineNames.Read, disruptionTimeout: TimeSpan.FromSeconds(3), disruptionInProgress: true, suggestedTimeout: null); + + var act = async () => await sut.ExecuteAsync(IgnoresTheToken, false, testContextAccessor.Current.CancellationToken); + + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task Outside_a_disruption_the_configured_timeout_still_applies() + { + var sut = CreateTimedSut(ResiliencePipelineNames.Read, disruptionTimeout: TimeSpan.FromSeconds(3), disruptionInProgress: false); + + var act = async () => await sut.ExecuteAsync(IgnoresTheToken, false, testContextAccessor.Current.CancellationToken); + + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task A_disruption_is_ignored_when_no_disruption_timeout_is_configured() + { + var sut = CreateTimedSut(ResiliencePipelineNames.Read, disruptionTimeout: null, disruptionInProgress: true); + + var act = async () => await sut.ExecuteAsync(IgnoresTheToken, false, testContextAccessor.Current.CancellationToken); + + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task The_tier_suggestion_stands_in_for_an_unconfigured_disruption_timeout() + { + var sut = CreateTimedSut(ResiliencePipelineNames.Read, disruptionInProgress: true, suggestedTimeout: TimeSpan.FromSeconds(3)); + + var result = await sut.ExecuteAsync(IgnoresTheToken, false, testContextAccessor.Current.CancellationToken); + + result.Should().BeTrue("the tier is already relaxing its own timeouts by that much"); + } + + [Fact] + public async Task A_configured_disruption_timeout_wins_over_the_suggestion() + { + var sut = CreateTimedSut( + ResiliencePipelineNames.Read, + disruptionTimeout: TimeSpan.FromSeconds(3), + disruptionInProgress: true, + suggestedTimeout: TimeSpan.FromMilliseconds(120)); + + var result = await sut.ExecuteAsync(IgnoresTheToken, false, testContextAccessor.Current.CancellationToken); + + result.Should().BeTrue(); + } + + [Fact] + public async Task A_suggestion_below_the_normal_timeout_never_shortens_it() + { + // The callback outlives the suggestion but not the configured timeout. + var sut = CreateTimedSut( + ResiliencePipelineNames.Read, + disruptionInProgress: true, + suggestedTimeout: TimeSpan.FromMilliseconds(1), + requestTimeout: TimeSpan.FromSeconds(3)); + + var result = await sut.ExecuteAsync(IgnoresTheToken, false, testContextAccessor.Current.CancellationToken); + + result.Should().BeTrue(); + } + public ValueTask DisposeAsync() { @@ -71,4 +192,118 @@ public ValueTask InitializeAsync() }); return ValueTask.CompletedTask; } + + [Fact] + public async Task Caller_cancellation_also_abandons_the_callback() + { + // A long timeout, so only the caller's token can release it. + var sut = CreateTimedSut(ResiliencePipelineNames.Read, requestTimeout: TimeSpan.FromSeconds(30)); + using var caller = new CancellationTokenSource(); + var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + // Cancelled once running, or Polly would refuse the call up front. + var pending = sut.ExecuteAsync( + async _ => + { + entered.TrySetResult(); + await Task.Delay(CallbackDuration, CancellationToken.None).ConfigureAwait(false); + return true; + }, + false, + caller.Token).AsTask(); + await entered.Task.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + + var elapsed = Stopwatch.StartNew(); + await caller.CancelAsync(); + var act = async () => await pending; + await act.Should().ThrowAsync("the caller is released rather than waiting out the callback"); + elapsed.Elapsed.Should().BeLessThan(CallbackDuration, "it did not wait for the callback to finish"); + } + + [Fact] + public async Task Caller_cancellation_waits_for_the_callback_on_the_write_pipeline() + { + var sut = CreateTimedSut(ResiliencePipelineNames.Write, requestTimeout: TimeSpan.FromSeconds(30)); + using var caller = new CancellationTokenSource(); + + var pending = sut.ExecuteAsync(IgnoresTheToken, false, caller.Token).AsTask(); + await caller.CancelAsync(); + + (await pending).Should().BeTrue("the callback ran to its own completion"); + } + + [Fact] + public async Task A_timed_out_read_is_not_retried() + { + var sut = CreateTimedSut(ResiliencePipelineNames.Read, retryCount: 1); + var attempts = 0; + + var act = async () => await sut.ExecuteAsync( + token => + { + Interlocked.Increment(ref attempts); + return IgnoresTheToken(token); + }, + false, + testContextAccessor.Current.CancellationToken); + + await act.Should().ThrowAsync(); + attempts.Should().Be(1); + } + + [Fact] + public async Task Caller_cancellation_does_not_open_the_circuit() + { + var sut = CreateTimedSut(ResiliencePipelineNames.Read, requestTimeout: TimeSpan.FromSeconds(30), exceptionsAllowedBeforeBreaking: 2); + for (var i = 0; i < 5; i++) + { + using var caller = new CancellationTokenSource(); + var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var pending = sut.ExecuteAsync( + async _ => + { + entered.TrySetResult(); + await Task.Delay(CallbackDuration, CancellationToken.None).ConfigureAwait(false); + return true; + }, + false, + caller.Token).AsTask(); + await entered.Task.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + await caller.CancelAsync(); + var act = async () => await pending; + await act.Should().ThrowAsync(); + } + + var result = await sut.ExecuteAsync(_ => new ValueTask(true), false, testContextAccessor.Current.CancellationToken); + + result.Should().BeTrue("an open circuit would have returned the fallback default instead"); + } + + private static ValueTask IgnoresTheToken(CancellationToken token) => RunIgnoringCancellation(); + + private static async ValueTask RunIgnoringCancellation() + { + await Task.Delay(CallbackDuration, CancellationToken.None).ConfigureAwait(false); + return true; + } + + private static ResiliencePipelineWrapper CreateTimedSut(string scope, TimeSpan? disruptionTimeout = null, bool disruptionInProgress = false, TimeSpan? suggestedTimeout = null, TimeSpan? requestTimeout = null, int retryCount = 0, int exceptionsAllowedBeforeBreaking = 500) + { + var options = new ResiliencePoliciesOptions + { + RequestTimeout = requestTimeout ?? PipelineTimeout, + RetryCount = retryCount, + ExceptionsAllowedBeforeBreaking = exceptionsAllowedBeforeBreaking, + TelemetryEnabled = false, + DisruptionRequestTimeout = disruptionTimeout, + }; + var monitor = Substitute.For>(); + monitor.CurrentValue.Returns(_ => options); + monitor.Get(Arg.Any()).Returns(_ => options); + var state = Substitute.For(); + state.InProgress.Returns(disruptionInProgress); + state.SuggestedTimeout.Returns(suggestedTimeout); + + return new(new ResiliencePipelineFactory(NullLoggerFactory.Instance, null, monitor, state), scope); + } }