ServiceStack / Issues

Issue Tracker for the commercial versions of ServiceStack
11 stars 8 forks source link

Redis Sentinel Failover #802

Closed dhanson-opentable closed 9 months ago

dhanson-opentable commented 10 months ago

Describe the issue

Unhandled exception when getting a ready-only client from PooledRedisClientManager when the first of multiple sentinel hosts is down.

Reproduction

https://github.com/ServiceStack/ServiceStack/pull/1330

This PR includes two parts:

  1. an integration test that shuts down the first of three sentinel hosts before attempting to get a read-only client and request a value from redis.
  2. a simple fix, nearly identical to how the GetMasterHost function is implemented.

Expected behavior

The library should catch the unhandled exception, trigger the error callback which will force the library to connect to the next available sentinel host.

System Info

.NET SDK:
 Version:   7.0.306
 Commit:    f500069cb7

Runtime Environment:
 OS Name:     ubuntu
 OS Version:  20.04
 OS Platform: Linux
 RID:         ubuntu.20.04-x64
 Base Path:   /usr/share/dotnet/sdk/7.0.306/

Host:
  Version:      7.0.9
  Architecture: x64
  Commit:       8e9a17b221

.NET SDKs installed:
  3.1.426 [/usr/share/dotnet/sdk]
  6.0.412 [/usr/share/dotnet/sdk]
  7.0.306 [/usr/share/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 3.1.32 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.20 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.9 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 3.1.32 [/usr/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.20 [/usr/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.9 [/usr/share/dotnet/shared/Microsoft.NETCore.App]

Additional context

No response

Validations

mythz commented 10 months ago

Thanks for the PR! Can you please accept the Contributor License Agreement so we can merge this, thanks.

mythz commented 9 months ago

Thanks, I've just merged the PR. We plan on releasing a new version on NuGet early next week.