Fix Redis TryAcquireAsync handling for disconnected databases - #283
#283Fix Redis TryAcquireAsync handling for disconnected databases#283teesofttech wants to merge 2 commits intomadelson:mastermadelson/DistributedLock:masterfrom teesofttech:fix/242-redis-unavailability-vs-lock-contentionteesofttech/DistributedLock:fix/242-redis-unavailability-vs-lock-contentionCopy head branch name to clipboard
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes Redis TryAcquireAsync behavior in RedLock when one or more IDatabase instances are disconnected (where StackExchange.Redis may backlog operations instead of failing fast), ensuring disconnected cases surface promptly and correctly without masking real underlying faults.
Changes:
- Update
RedLockAcquire.WaitForAcquireAsyncto properly propagate the synthetic “disconnected database” fault when it becomes decisive and no real faulted/canceled acquire tasks exist. - Adjust the disconnected-database decisiveness logic so a single disconnected database is immediately decisive in the single-server scenario.
- Add CI tests covering: single-db disconnect throws, synthetic disconnect does not mask a real fault, and single-db contention returns
null.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/DistributedLock.Tests/Tests/Redis/RedisDistributedLockTest.cs | Adds targeted CI tests validating TryAcquireAsync outcomes for disconnected and contended Redis database scenarios. |
| src/DistributedLock.Redis/RedLock/RedLockAcquire.cs | Fixes fault propagation for synthetic disconnect faults and refines decisiveness logic for single-database acquisition. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
If you can help review this PR as well please @madelson |
| .ToArray(); | ||
| if (faultingTasks.Length == 0) | ||
| { | ||
| await completed.ConfigureAwait(false); // propagate a synthetic disconnected fault |
There was a problem hiding this comment.
How does this help and what is a synthetic disconnected fault?
There was a problem hiding this comment.
The original Redis operation is still waiting in the backlog, so it doesn’t appear in faultingTasks. When we detect that Redis is disconnected, we create a separate faulted task and assign it to completed. Awaiting it ensures the caller gets the useful RedisException rather than an empty AggregateException. I’ve updated the code comment to make that clearer.
No description provided.