diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index 326c50e16..9cbce2f96 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -1,5 +1,9 @@ # Release Notes +## Unreleased + +- Fix [#1988](https://github.com/StackExchange/StackExchange.Redis/issues/1988): Don't issue `SELECT` commands if explicitly disabled ([#2023 by NickCraver](https://github.com/StackExchange/StackExchange.Redis/pull/2023)) + ## 2.5.43 - Adds: Bounds checking for `ExponentialRetry` backoff policy ([#1921 by gliljas](https://github.com/StackExchange/StackExchange.Redis/pull/1921)) diff --git a/src/StackExchange.Redis/PhysicalBridge.cs b/src/StackExchange.Redis/PhysicalBridge.cs index 92d2f83f7..e39cad16a 100644 --- a/src/StackExchange.Redis/PhysicalBridge.cs +++ b/src/StackExchange.Redis/PhysicalBridge.cs @@ -1372,7 +1372,7 @@ private WriteResult WriteMessageToServerInsideWriteLock(PhysicalConnection conne LastCommand = cmd; bool isMasterOnly = message.IsMasterOnly(); - if (isMasterOnly && ServerEndPoint.IsReplica && (ServerEndPoint.ReplicaReadOnly || !ServerEndPoint.AllowReplicaWrites)) + if (isMasterOnly && !ServerEndPoint.SupportsPrimaryWrites) { throw ExceptionFactory.MasterOnly(Multiplexer.IncludeDetailInExceptions, message.Command, message, ServerEndPoint); } @@ -1447,7 +1447,10 @@ private WriteResult WriteMessageToServerInsideWriteLock(PhysicalConnection conne case RedisCommand.UNKNOWN: case RedisCommand.DISCARD: case RedisCommand.EXEC: - connection.SetUnknownDatabase(); + if (ServerEndPoint.SupportsDatabases) + { + connection.SetUnknownDatabase(); + } break; } return WriteResult.Success; diff --git a/src/StackExchange.Redis/PhysicalConnection.cs b/src/StackExchange.Redis/PhysicalConnection.cs index 4b32ecfee..f699d9055 100644 --- a/src/StackExchange.Redis/PhysicalConnection.cs +++ b/src/StackExchange.Redis/PhysicalConnection.cs @@ -573,7 +573,8 @@ internal Message GetSelectDatabaseCommand(int targetDatabase, Message message) } int available = serverEndpoint.Databases; - if (!serverEndpoint.HasDatabases) // only db0 is available on cluster/twemproxy/envoyproxy + // Only db0 is available on cluster/twemproxy/envoyproxy + if (!serverEndpoint.SupportsDatabases) { if (targetDatabase != 0) { // should never see this, since the API doesn't allow it; thus not too worried about ExceptionFactory diff --git a/src/StackExchange.Redis/ServerEndPoint.cs b/src/StackExchange.Redis/ServerEndPoint.cs index 2aaf6a63e..e87e2a73e 100755 --- a/src/StackExchange.Redis/ServerEndPoint.cs +++ b/src/StackExchange.Redis/ServerEndPoint.cs @@ -32,9 +32,9 @@ internal sealed partial class ServerEndPoint : IDisposable private int databases, writeEverySeconds; private PhysicalBridge interactive, subscription; - private bool isDisposed; + private bool isDisposed, replicaReadOnly, isReplica, allowReplicaWrites; + private bool? supportsDatabases, supportsPrimaryWrites; private ServerType serverType; - private bool replicaReadOnly, isReplica; private volatile UnselectableFlags unselectableReasons; private Version version; @@ -76,8 +76,17 @@ public ServerEndPoint(ConnectionMultiplexer multiplexer, EndPoint endpoint) public int Databases { get { return databases; } set { SetConfig(ref databases, value); } } public EndPoint EndPoint { get; } + /// + /// Whether this endpoint supports databases at all. + /// Note that some servers are cluster but present as standalone (e.g. Redis Enterprise), so we respect + /// being disabled here as a performance workaround. + /// + /// + /// This is memoized because it's accessed on hot paths inside the write lock. + /// + public bool SupportsDatabases => + supportsDatabases ??= (serverType == ServerType.Standalone && Multiplexer.RawConfig.CommandMap.IsAvailable(RedisCommand.SELECT)); - public bool HasDatabases => serverType == ServerType.Standalone; public bool IsConnected => interactive?.IsConnected == true; public bool IsSubscriberConnected => subscription?.IsConnected == true; @@ -85,6 +94,7 @@ public ServerEndPoint(ConnectionMultiplexer multiplexer, EndPoint endpoint) public bool SupportsSubscriptions => Multiplexer.CommandMap.IsAvailable(RedisCommand.SUBSCRIBE); public bool IsConnecting => interactive?.IsConnecting == true; + public bool SupportsPrimaryWrites => supportsPrimaryWrites ??= (!IsReplica || !ReplicaReadOnly || AllowReplicaWrites); private readonly List> _pendingConnectionMonitors = new List>(); @@ -176,7 +186,15 @@ public bool ReplicaReadOnly set => SetConfig(ref replicaReadOnly, value); } - public bool AllowReplicaWrites { get; set; } + public bool AllowReplicaWrites + { + get => allowReplicaWrites; + set + { + allowReplicaWrites = value; + ClearMemoized(); + } + } public Version Version { @@ -946,10 +964,17 @@ private void SetConfig(ref T field, T value, [CallerMemberName] string caller { Multiplexer.Trace(caller + " changed from " + field + " to " + value, "Configuration"); field = value; + ClearMemoized(); Multiplexer.ReconfigureIfNeeded(EndPoint, false, caller); } } + private void ClearMemoized() + { + supportsDatabases = null; + supportsPrimaryWrites = null; + } + /// /// For testing only ///