diff --git a/libs/cluster/Server/ClusterConfig.cs b/libs/cluster/Server/ClusterConfig.cs index 046e92d8da1..f85905ed7da 100644 --- a/libs/cluster/Server/ClusterConfig.cs +++ b/libs/cluster/Server/ClusterConfig.cs @@ -168,22 +168,22 @@ public bool HasAssignedSlots(ushort workerId) /// 3. Local slots for a replica are those slots served by its primary only for read operations /// /// Slot to check - /// Used to override write restrictions for non-local slots that are replicas of the slot owner + /// Whether a replica can serve reads for slots owned by its primary /// True if slot is owned by this node, false otherwise [MethodImpl(MethodImplOptions.AggressiveInlining)] - public bool IsLocal(ushort slot, bool readWriteSession = true) - => slotMap[slot].workerId == LOCAL_WORKER_ID || IsLocalExpensive(slot, readWriteSession); + public bool IsLocal(ushort slot, bool enableReplicaReads = true) + => slotMap[slot].workerId == LOCAL_WORKER_ID || IsLocalExpensive(slot, enableReplicaReads); /// /// If slot in MIGRATE state then it must have been set by original owner, so we keep treating it like a local slot and serve requests if the key has not yet migrated. /// If it is a read command and this is a replica the associated slot should be assigned to this node's primary in order for the read request to be served. /// /// - /// + /// /// - private bool IsLocalExpensive(ushort slot, bool readWriteSession) + private bool IsLocalExpensive(ushort slot, bool enableReplicaReads) => slotMap[slot]._state == SlotState.MIGRATING || - (readWriteSession && + (enableReplicaReads && workers[1].Role == NodeRole.REPLICA && slotMap[slot]._workerId > 1 && LocalNodePrimaryId != null && diff --git a/libs/cluster/Server/ClusterManagerSlotState.cs b/libs/cluster/Server/ClusterManagerSlotState.cs index 17c1631ebd3..5455af216be 100644 --- a/libs/cluster/Server/ClusterManagerSlotState.cs +++ b/libs/cluster/Server/ClusterManagerSlotState.cs @@ -229,7 +229,7 @@ public bool TryPrepareSlotForImport(int slot, string nodeid, out ReadOnlySpan slots, string nodeid, out Read foreach (var slot in slots) { // Can only import remote slots - if (current.IsLocal((ushort)slot, readWriteSession: false)) + if (current.IsLocal((ushort)slot, enableReplicaReads: false)) { errorMessage = Encoding.ASCII.GetBytes($"ERR This is a local hash slot {slot} and is already imported"); return false; diff --git a/libs/cluster/Server/Replication/ReplicaOps/ReplicaDiskbasedSync.cs b/libs/cluster/Server/Replication/ReplicaOps/ReplicaDiskbasedSync.cs index f092c3d2959..ee24d4e6e16 100644 --- a/libs/cluster/Server/Replication/ReplicaOps/ReplicaDiskbasedSync.cs +++ b/libs/cluster/Server/Replication/ReplicaOps/ReplicaDiskbasedSync.cs @@ -346,7 +346,7 @@ public AofAddress TryReplicaDiskbasedRecovery( // Mark this txn run as a read-write session if we are replaying as a replica // This is necessary to ensure that the stored procedure can perform write operations if needed - clusterProvider.replicationManager.aofProcessor.SetReadWriteSession(); + clusterProvider.replicationManager.aofProcessor.SetInternalWriteSession(); return this.replicationOffset; } diff --git a/libs/cluster/Server/Replication/ReplicaOps/ReplicaDisklessSync.cs b/libs/cluster/Server/Replication/ReplicaOps/ReplicaDisklessSync.cs index 081711e708f..9fedb778920 100644 --- a/libs/cluster/Server/Replication/ReplicaOps/ReplicaDisklessSync.cs +++ b/libs/cluster/Server/Replication/ReplicaOps/ReplicaDisklessSync.cs @@ -245,7 +245,7 @@ public AofAddress TryReplicaDisklessRecovery(SyncMetadata primarySyncMetadata, o // Mark this txn run as a read-write session if we are replaying as a replica // This is necessary to ensure that the stored procedure can perform write operations if needed - clusterProvider.replicationManager.aofProcessor.SetReadWriteSession(); + clusterProvider.replicationManager.aofProcessor.SetInternalWriteSession(); return this.replicationOffset; } diff --git a/libs/cluster/Session/ClusterSession.cs b/libs/cluster/Session/ClusterSession.cs index d79123c0ce3..e8ed4a6e755 100644 --- a/libs/cluster/Session/ClusterSession.cs +++ b/libs/cluster/Session/ClusterSession.cs @@ -34,15 +34,15 @@ internal sealed partial class ClusterSession : IClusterSession public long LocalCurrentEpoch => _localCurrentEpoch; - /// - /// Indicates if this is a session that allows for reads and writes - /// - bool readWriteSession = false; + bool readOnlySession; + bool internalWriteSession; - public bool ReadWriteSession => clusterProvider.clusterManager.CurrentConfig.IsPrimary || readWriteSession; + public bool ReadOnlySession => readOnlySession; + public bool IsInternalWriteSession => internalWriteSession; - public void SetReadOnlySession() => readWriteSession = false; - public void SetReadWriteSession() => readWriteSession = true; + public void SetReadOnlySession() => readOnlySession = true; + public void SetReadWriteSession() => readOnlySession = false; + public void SetInternalWriteSession() => internalWriteSession = true; /// public bool IsReplicating { get; private set; } diff --git a/libs/cluster/Session/MigrateCommand.cs b/libs/cluster/Session/MigrateCommand.cs index 0fbed41096d..fc749f96c1c 100644 --- a/libs/cluster/Session/MigrateCommand.cs +++ b/libs/cluster/Session/MigrateCommand.cs @@ -195,7 +195,7 @@ private bool NetworkTryMIGRATE(out bool invalidParameters) // Check if all keys are local R/W because we migrate keys and need to be able to delete them var slot = HashSlotUtils.HashSlot(currKeySlice); - if (!current.IsLocal(slot, readWriteSession: false)) + if (!current.IsLocal(slot, enableReplicaReads: false)) { pstate = MigrateCmdParseState.SLOTNOTLOCAL; continue; @@ -247,7 +247,7 @@ private bool NetworkTryMIGRATE(out bool invalidParameters) } // Check if slot is local and can be migrated - if (!current.IsLocal((ushort)slot, readWriteSession: false)) + if (!current.IsLocal((ushort)slot, enableReplicaReads: false)) { pstate = MigrateCmdParseState.SLOTNOTLOCAL; slotParseError = slot; @@ -299,7 +299,7 @@ private bool NetworkTryMIGRATE(out bool invalidParameters) } // Check if slot is not owned by current node or cluster mode is not enabled - if (!current.IsLocal((ushort)slot, readWriteSession: false)) + if (!current.IsLocal((ushort)slot, enableReplicaReads: false)) { pstate = MigrateCmdParseState.SLOTNOTLOCAL; slotParseError = slot; diff --git a/libs/cluster/Session/SlotVerification/ClusterSlotVerify.cs b/libs/cluster/Session/SlotVerification/ClusterSlotVerify.cs index b4401764c74..9759ad659b3 100644 --- a/libs/cluster/Session/SlotVerification/ClusterSlotVerify.cs +++ b/libs/cluster/Session/SlotVerification/ClusterSlotVerify.cs @@ -26,7 +26,7 @@ private ClusterSlotVerificationResult SingleKeySlotVerify(ref ClusterConfig conf ClusterSlotVerificationResult SingleKeyReadSlotVerify(ref ClusterConfig config, ref PinnedSpanByte keySlice) { var _slot = slot == -1 ? HashSlotUtils.HashSlot(keySlice) : (ushort)slot; - var IsLocal = config.IsLocal(_slot); + var IsLocal = config.IsLocal(_slot, enableReplicaReads: readOnlySession); var state = config.GetState(_slot); // If local check we can serve request or redirect with ask @@ -69,7 +69,7 @@ ClusterSlotVerificationResult SingleKeyReadWriteSlotVerify(bool waitForStableSlo var _slot = slot == -1 ? HashSlotUtils.HashSlot(keySlice) : (ushort)slot; tryAgain: - var IsLocal = config.IsLocal(_slot, readWriteSession: readWriteSession); + var IsLocal = config.IsLocal(_slot, enableReplicaReads: internalWriteSession); var state = config.GetState(_slot); if (waitForStableSlot && state is SlotState.IMPORTING or SlotState.MIGRATING) @@ -79,7 +79,7 @@ ClusterSlotVerificationResult SingleKeyReadWriteSlotVerify(bool waitForStableSlo } // Redirect r/w requests towards primary - if (config.LocalNodeRole == NodeRole.REPLICA && !readWriteSession) + if (config.LocalNodeRole == NodeRole.REPLICA && !internalWriteSession) return new(SlotVerifiedState.MOVED, _slot); if (IsLocal) diff --git a/libs/resources/RespCommandsDocs.json b/libs/resources/RespCommandsDocs.json index 91d3bd4cf49..19896d049a8 100644 --- a/libs/resources/RespCommandsDocs.json +++ b/libs/resources/RespCommandsDocs.json @@ -6130,7 +6130,7 @@ { "Command": "READWRITE", "Name": "READWRITE", - "Summary": "Enables read-write queries for a connection to a Reids Cluster replica node.", + "Summary": "Disables read queries for a connection to a Redis Cluster replica node.", "Group": "Cluster", "Complexity": "O(1)" }, diff --git a/libs/server/AOF/AofProcessor.cs b/libs/server/AOF/AofProcessor.cs index 6c652e5fd6b..2fe811efb31 100644 --- a/libs/server/AOF/AofProcessor.cs +++ b/libs/server/AOF/AofProcessor.cs @@ -93,14 +93,14 @@ public sealed unsafe partial class AofProcessor RangeIndexManager activeRangeIndexManager; /// - /// Set ReadWriteSession on the cluster session (NOTE: used for replaying stored procedures only) + /// Allow the cluster session to apply writes while replaying stored procedures /// - public void SetReadWriteSession() + public void SetInternalWriteSession() { for (var i = 0; i < storeWrapper.serverOptions.AofVirtualSublogCount; i++) { var respServerSession = aofReplayCoordinator.GetReplayContext(i).respServerSession; - respServerSession.clusterSession.SetReadWriteSession(); + respServerSession.clusterSession.SetInternalWriteSession(); } } diff --git a/libs/server/Cluster/IClusterSession.cs b/libs/server/Cluster/IClusterSession.cs index 7f1929bb71b..b986c317502 100644 --- a/libs/server/Cluster/IClusterSession.cs +++ b/libs/server/Cluster/IClusterSession.cs @@ -18,9 +18,14 @@ public interface IClusterSession string RemoteNodeId { get; } /// - /// Type of session + /// Whether this connection may serve read-only commands from a replica /// - bool ReadWriteSession { get; } + bool ReadOnlySession { get; } + + /// + /// Whether this internal session may apply writes while replaying the AOF + /// + bool IsInternalWriteSession { get; } /// /// If the current session is part of an active replication stream (set on first APPENDLOG, including the init handshake). @@ -35,15 +40,20 @@ public interface IClusterSession IGarnetServer Server { get; set; } /// - /// Make this cluster session a read-only session + /// Allow this connection to serve read-only commands from a replica /// void SetReadOnlySession(); /// - /// Make this cluster session a read-write session + /// Restore the default behavior of redirecting commands from a replica /// void SetReadWriteSession(); + /// + /// Allow the internal AOF replay session to apply writes on a replica + /// + void SetInternalWriteSession(); + /// /// Local current epoch /// diff --git a/libs/server/Config/RuntimeServerConfig.cs b/libs/server/Config/RuntimeServerConfig.cs index c5703dd2b6b..c4ba3e135a1 100644 --- a/libs/server/Config/RuntimeServerConfig.cs +++ b/libs/server/Config/RuntimeServerConfig.cs @@ -114,9 +114,8 @@ void SetReadOnly(ServerConfigType t, string name, ConfigKind kind, // rejects them because they are constants or physical (require restart). Their CONFIG GET value // is computed by the per-option formatter, which reads directly from the startup // GarnetServerOptions (the read-only fall-through) — no runtime slot is used. - // NOTE: slave-read-only is intentionally NOT here: it is a per-session value (READWRITE/READONLY, - // https://redis.io/docs/latest/commands/readwrite/) and is handled directly by the CONFIG GET - // handler, which has the calling session in scope. + // NOTE: slave-read-only is intentionally not here because it is a fixed compatibility setting + // handled directly by CONFIG GET. SetReadOnly(ServerConfigType.TIMEOUT, "timeout", ConfigKind.Int32 | ConfigKind.Seconds | ConfigKind.TimeSpan, static _ => "0", ConfigTimeUnit.Seconds); SetReadOnly(ServerConfigType.SAVE, "save", ConfigKind.String, static _ => ""); diff --git a/libs/server/Resp/BasicCommands.cs b/libs/server/Resp/BasicCommands.cs index 8b25229045b..dc3ab12d991 100644 --- a/libs/server/Resp/BasicCommands.cs +++ b/libs/server/Resp/BasicCommands.cs @@ -999,7 +999,7 @@ private bool NetworkFLUSHDB() return AbortWithWrongNumberOfArguments(nameof(RespCommand.FLUSHDB)); } - if (storeWrapper.serverOptions.EnableCluster && storeWrapper.clusterProvider.IsReplica() && !clusterSession.ReadWriteSession) + if (storeWrapper.serverOptions.EnableCluster && storeWrapper.clusterProvider.IsReplica() && !clusterSession.IsInternalWriteSession) { while (!RespWriteUtils.TryWriteError(CmdStrings.RESP_ERR_FLUSHALL_READONLY_REPLICA, ref dcurr, dend)) SendAndReset(); @@ -1021,7 +1021,7 @@ private bool NetworkFLUSHALL() return AbortWithWrongNumberOfArguments(nameof(RespCommand.FLUSHALL)); } - if (storeWrapper.serverOptions.EnableCluster && storeWrapper.clusterProvider.IsReplica() && !clusterSession.ReadWriteSession) + if (storeWrapper.serverOptions.EnableCluster && storeWrapper.clusterProvider.IsReplica() && !clusterSession.IsInternalWriteSession) { return AbortWithErrorMessage(CmdStrings.RESP_ERR_FLUSHALL_READONLY_REPLICA); } @@ -1034,7 +1034,7 @@ private bool NetworkFLUSHALL() } /// - /// Mark this session as readonly session + /// Allow this connection to serve read-only commands from a replica /// /// private bool NetworkREADONLY() @@ -1047,7 +1047,7 @@ private bool NetworkREADONLY() } /// - /// Mark this session as readwrite + /// Restore the default behavior of redirecting commands from a replica /// /// private bool NetworkREADWRITE() diff --git a/libs/server/ServerConfig.cs b/libs/server/ServerConfig.cs index 48f7b56d526..fb10dbdef88 100644 --- a/libs/server/ServerConfig.cs +++ b/libs/server/ServerConfig.cs @@ -18,8 +18,7 @@ public static ServerConfigType GetConfig(Span parameter) if (parameter.SequenceEqual("*"u8)) return ServerConfigType.ALL; - // slave-read-only is a per-session value (READWRITE/READONLY) and is resolved by the CONFIG GET - // handler which has the session in scope; it is not part of the runtime config table. + // slave-read-only is a fixed compatibility setting handled directly by CONFIG GET. if (parameter.SequenceEqual("SLAVE-READ-ONLY"u8)) return ServerConfigType.SLAVE_READ_ONLY; @@ -51,7 +50,7 @@ private unsafe bool NetworkCONFIG_GET() if (serverConfigType == ServerConfigType.ALL) { parameters = [.. RuntimeServerConfig.RuntimeTypes]; - // slave-read-only is session-scoped and not part of the table, so include it explicitly. + // slave-read-only is not part of the runtime table, so include it explicitly. parameters.Add(ServerConfigType.SLAVE_READ_ONLY); returnAll = true; continue; @@ -74,10 +73,8 @@ private unsafe bool NetworkCONFIG_GET() string name, value; if (configType == ServerConfigType.SLAVE_READ_ONLY) { - // Per-session value: a session is read-only only when it is on a replica and has not - // opted into writes via READWRITE (see https://redis.io/docs/latest/commands/readwrite/). name = "slave-read-only"; - value = clusterSession == null || clusterSession.ReadWriteSession ? "no" : "yes"; + value = "yes"; } else { diff --git a/test/cluster/Garnet.test.cluster.replication/ReplicationTests/ClusterReplicationBaseTests.cs b/test/cluster/Garnet.test.cluster.replication/ReplicationTests/ClusterReplicationBaseTests.cs index 8fefccb8a43..60ad23ef8cf 100644 --- a/test/cluster/Garnet.test.cluster.replication/ReplicationTests/ClusterReplicationBaseTests.cs +++ b/test/cluster/Garnet.test.cluster.replication/ReplicationTests/ClusterReplicationBaseTests.cs @@ -291,6 +291,45 @@ public void ClusterCheckpointRetrieveDisableStorageTier([Values] bool performRMW ClusterSRPrimaryCheckpointRetrieve(performRMW, disableObjects, false, true); } + [Test, Order(5)] + [Category("REPLICATION")] + public void ReadWriteSessionDoesNotEnableReplicaWritesAfterFailover() + { + const int primaryIndex = 0; + const int replicaIndex = 1; + const string key = "readwrite-failover-key"; + const string value = "value"; + + context.CreateInstances(2, enableAOF: true, useTLS: useTLS, asyncReplay: asyncReplay, sublogCount: sublogCount); + context.CreateConnection(useTLS: useTLS); + _ = context.clusterTestUtils.SimpleSetupCluster(primary_count: 1, replica_count: 1, logger: context.logger); + + using var session = context.clusterTestUtils.CreateGarnetClientSession(primaryIndex, useTLS: useTLS); + session.Connect(); + ClassicAssert.AreEqual("OK", session.ExecuteAsync("READWRITE").GetAwaiter().GetResult()); + ClassicAssert.AreEqual("OK", session.ExecuteAsync("SET", key, value).GetAwaiter().GetResult()); + context.clusterTestUtils.WaitForReplicaAofSync(primaryIndex, replicaIndex, context.logger); + + _ = context.clusterTestUtils.ClusterFailover(replicaIndex, logger: context.logger); + context.clusterTestUtils.WaitForNoFailover(replicaIndex, context.logger); + context.clusterTestUtils.WaitForFailoverCompleted(replicaIndex, context.logger); + context.clusterTestUtils.WaitForReplicaRecovery(primaryIndex, context.logger); + context.clusterTestUtils.WaitForReplicaAofSync(replicaIndex, primaryIndex, context.logger); + + var slot = ClusterTestUtils.HashSlot(Encoding.ASCII.GetBytes(key)); + var exception = Assert.Throws(() => session.ExecuteAsync("GET", key).GetAwaiter().GetResult()); + StringAssert.StartsWith($"MOVED {slot} ", exception.Message); + + exception = Assert.Throws(() => session.ExecuteAsync("SET", key, "local-value").GetAwaiter().GetResult()); + StringAssert.StartsWith($"MOVED {slot} ", exception.Message); + + exception = Assert.Throws(() => session.ExecuteAsync("FLUSHALL").GetAwaiter().GetResult()); + ClassicAssert.AreEqual("ERR You can't write against a read only replica.", exception.Message); + + ClassicAssert.AreEqual("OK", session.ExecuteAsync("READONLY").GetAwaiter().GetResult()); + ClassicAssert.AreEqual(value, session.ExecuteAsync("GET", key).GetAwaiter().GetResult()); + } + [Test, Order(6)] [Category("REPLICATION")] public void ClusterSRPrimaryCheckpointRetrieve([Values] bool performRMW, [Values] bool disableObjects, [Values] bool manySegments) diff --git a/test/cluster/Garnet.test.cluster/ClusterRedirectTests.cs b/test/cluster/Garnet.test.cluster/ClusterRedirectTests.cs index 7eb32279156..9ca2c9492cb 100644 --- a/test/cluster/Garnet.test.cluster/ClusterRedirectTests.cs +++ b/test/cluster/Garnet.test.cluster/ClusterRedirectTests.cs @@ -1060,5 +1060,41 @@ public void ClusterUnknownEndpointPreferredTests() connections.ToList().ForEach(x => x.Dispose()); context.logger.LogDebug("1. ClusterUnknownEndpointPreferredTests done"); } + + [Test, Order(6)] + [Category("CLUSTER")] + public void ClusterReplicaReadModeTests() + { + const int primaryIndex = 0; + const int replicaIndex = 1; + const string key = "replica-read-mode-key"; + const string value = "value"; + + context.CreateInstances(2, enableAOF: true); + context.CreateConnection(); + _ = context.clusterTestUtils.SimpleSetupCluster(primary_count: 1, replica_count: 1, logger: context.logger); + + var keyBytes = Encoding.ASCII.GetBytes(key); + var slot = ClusterTestUtils.HashSlot(keyBytes); + var response = context.clusterTestUtils.SetKey(primaryIndex, keyBytes, Encoding.ASCII.GetBytes(value), out _, out _, logger: context.logger); + ClassicAssert.AreEqual(ResponseState.OK, response); + context.clusterTestUtils.WaitForReplicaAofSync(primaryIndex, replicaIndex, context.logger); + + using var replicaClient = context.clusterTestUtils.CreateGarnetClientSession(replicaIndex); + replicaClient.Connect(); + + var exception = Assert.Throws(() => replicaClient.ExecuteAsync("GET", key).GetAwaiter().GetResult()); + StringAssert.StartsWith($"MOVED {slot} ", exception.Message); + + ClassicAssert.AreEqual("OK", replicaClient.ExecuteAsync("READONLY").GetAwaiter().GetResult()); + ClassicAssert.AreEqual(value, replicaClient.ExecuteAsync("GET", key).GetAwaiter().GetResult()); + + exception = Assert.Throws(() => replicaClient.ExecuteAsync("SET", key, "local-value").GetAwaiter().GetResult()); + StringAssert.StartsWith($"MOVED {slot} ", exception.Message); + + ClassicAssert.AreEqual("OK", replicaClient.ExecuteAsync("READWRITE").GetAwaiter().GetResult()); + exception = Assert.Throws(() => replicaClient.ExecuteAsync("GET", key).GetAwaiter().GetResult()); + StringAssert.StartsWith($"MOVED {slot} ", exception.Message); + } } } \ No newline at end of file diff --git a/test/cluster/Garnet.test.cluster/ClusterTestUtils.cs b/test/cluster/Garnet.test.cluster/ClusterTestUtils.cs index 0676e5cecb9..4864225d849 100644 --- a/test/cluster/Garnet.test.cluster/ClusterTestUtils.cs +++ b/test/cluster/Garnet.test.cluster/ClusterTestUtils.cs @@ -784,7 +784,10 @@ public async Task ConnectAsync(bool cluster = true, ILogger logger = null) { await InitMultiplexerAsync(GetRedisConfig(endpoints), textWriter, logger: logger); if (cluster) + { this.nodeIds = await GetNodeIdsAsync(logger: logger).ConfigureAwait(false); + await EnableReplicaReadsAsync(endpoints, logger).ConfigureAwait(false); + } } private async Task InitMultiplexerAsync(ConfigurationOptions redisConfig, TextWriter textWriter, bool failAssert = true, ILogger logger = null) @@ -855,7 +858,7 @@ public async Task GetNodeIdsAsync(List nodes = null, ILogger logg return nodeIds; } - public async Task ReconnectAsync(List nodes = null, TextWriter textWriter = null, ILogger logger = null) + public async Task ReconnectAsync(List nodes = null, TextWriter textWriter = null, ILogger logger = null, bool cluster = true) { await CloseConnectionsAsync().ConfigureAwait(false); var endPoints = endpoints; @@ -871,6 +874,18 @@ public async Task ReconnectAsync(List nodes = null, TextWriter textWriter = var connOpts = GetRedisConfig(endPoints); await InitMultiplexerAsync(connOpts, textWriter, logger: logger).ConfigureAwait(false); nodeIds = await GetNodeIdsAsync(nodes, logger).ConfigureAwait(false); + if (cluster) + await EnableReplicaReadsAsync(endPoints, logger).ConfigureAwait(false); + } + + private async Task EnableReplicaReadsAsync(EndPointCollection endPoints, ILogger logger) + { + foreach (var endPoint in endPoints) + { + logger?.LogInformation("({endpoint}) > READONLY", endPoint); + var result = await redis.GetServer(endPoint).ExecuteAsync("READONLY", Array.Empty(), CommandFlags.NoRedirect).ConfigureAwait(false); + ClassicAssert.AreEqual("OK", (string)result); + } } public EndPointCollection GetEndPoints() => endpoints; diff --git a/test/standalone/Garnet.test/RespAdminCommandsTests.cs b/test/standalone/Garnet.test/RespAdminCommandsTests.cs index 80e6ff7ef54..7950e0393b6 100644 --- a/test/standalone/Garnet.test/RespAdminCommandsTests.cs +++ b/test/standalone/Garnet.test/RespAdminCommandsTests.cs @@ -685,7 +685,7 @@ public async Task SeFlushDbAndFlushAllTest2([Values(RespCommand.FLUSHALL, RespCo [TestCase("timeout", "0")] [TestCase("save", "")] [TestCase("appendonly", "no")] - [TestCase("slave-read-only", "no")] + [TestCase("slave-read-only", "yes")] [TestCase("databases", "16")] [TestCase("cluster-node-timeout", "60")] public void SimpleConfigGet(string parameter, string parameterValue) diff --git a/test/standalone/Garnet.test/RespConfigTests.cs b/test/standalone/Garnet.test/RespConfigTests.cs index 2cc6fd42c65..0c90f4b61f1 100644 --- a/test/standalone/Garnet.test/RespConfigTests.cs +++ b/test/standalone/Garnet.test/RespConfigTests.cs @@ -913,8 +913,8 @@ public void ConfigSetRuntimeOptionValidationTest() /// /// Verifies that read-only parameters exposed through the runtime config table (timeout, save, - /// appendonly, databases) reject CONFIG SET, and that CONFIG GET * includes both the read-only - /// parameters and the per-session slave-read-only value. + /// appendonly, databases) reject CONFIG SET, and that CONFIG GET * includes the fixed + /// slave-read-only compatibility setting. /// [Test] public void ConfigGetAllAndReadOnlyRejectionTest() @@ -930,7 +930,7 @@ public void ConfigGetAllAndReadOnlyRejectionTest() var timeout = Assert.Throws(() => db.Execute("CONFIG", "SET", "timeout", "10")); ClassicAssert.AreEqual("ERR Option 'timeout' is read-only and cannot be set at runtime.", timeout.Message); - // CONFIG GET * returns a name/value map including read-only and per-session parameters. + // CONFIG GET * returns a name/value map including read-only parameters and compatibility settings. var all = (RedisResult[])db.Execute("CONFIG", "GET", "*"); ClassicAssert.IsTrue(all.Length % 2 == 0); var map = new Dictionary(StringComparer.OrdinalIgnoreCase); @@ -942,9 +942,9 @@ public void ConfigGetAllAndReadOnlyRejectionTest() ClassicAssert.IsTrue(map.ContainsKey("save")); ClassicAssert.IsTrue(map.ContainsKey("appendonly")); ClassicAssert.IsTrue(map.ContainsKey("databases")); - // Per-session parameter. + // Fixed compatibility setting. ClassicAssert.IsTrue(map.ContainsKey("slave-read-only")); - ClassicAssert.AreEqual("no", map["slave-read-only"]); + ClassicAssert.AreEqual("yes", map["slave-read-only"]); // A settable runtime parameter. ClassicAssert.IsTrue(map.ContainsKey("replica-sync-delay")); } diff --git a/website/docs/cluster/replication.md b/website/docs/cluster/replication.md index 5b290903202..4a81d2868fb 100644 --- a/website/docs/cluster/replication.md +++ b/website/docs/cluster/replication.md @@ -113,29 +113,24 @@ Currently, we do not support chained replication. ## Querying a Replica -By default replicas only serve read queries but can also be configure to process write requests. -This option is available by issuing once ```READWRITE``` command just before executing any write operation in a single client session. -Issuing ```READONLY``` will toggle back to serving read queries. +By default, a replica redirects commands to the primary that owns the requested hash slot. Clients can issue `READONLY` to allow the current connection to serve read commands directly from the replica for slots owned by its primary. These reads may return stale data while replication catches up. -If a replica is set to process read-only queries, it will respond with *-MOVED* to any write requests, redirecting them to the primary that is replicating. +Write commands are always redirected to the primary, including on connections that issued `READONLY`. The `READWRITE` command clears the connection's read-only mode and restores the default behavior of redirecting reads to the primary; it does not enable writes on the replica. ```bash -PS C:\Dev> redis-cli -h 192.168.1.26 -p 7001 -c -192.168.1.26:7001> set x 1234 --> Redirected to slot [16287] located at 192.168.1.26:7000 -OK -192.168.1.26:7000> set x 1234 +PS C:\Dev> redis-cli -h 192.168.1.26 -p 7001 +192.168.1.26:7001> get x +(error) MOVED 16287 192.168.1.26:7000 +192.168.1.26:7001> readonly OK -192.168.1.26:7000> get x -"1234" -192.168.1.26:7000> exit -PS C:\Dev> redis-cli -h 192.168.1.26 -p 7001 -c 192.168.1.26:7001> get x "1234" -192.168.1.26:7001> exit -PS C:\Dev> redis-cli -h 192.168.1.26 -p 7002 -192.168.1.26:7002> get x -"1234" +192.168.1.26:7001> set x 5678 +(error) MOVED 16287 192.168.1.26:7000 +192.168.1.26:7001> readwrite +OK +192.168.1.26:7001> get x +(error) MOVED 16287 192.168.1.26:7000 ``` ## Checkpointing & Recovery diff --git a/website/docs/commands/cluster.md b/website/docs/commands/cluster.md index 854dd8a58e1..f6303c22bb1 100644 --- a/website/docs/commands/cluster.md +++ b/website/docs/commands/cluster.md @@ -666,6 +666,8 @@ Simple string reply: OK. Enables read queries for a connection to a Redis Cluster replica node. +The replica can serve reads for slots owned by its primary, but the returned data may be stale. Write commands continue to redirect to the primary. + #### RESP Reply Simple string reply: OK. @@ -681,6 +683,8 @@ Simple string reply: OK. Disables read queries for a connection to a Redis Cluster replica node. +This command clears the connection's read-only mode and restores the default behavior of redirecting reads to the primary. It does not enable writes on the replica. + #### RESP Reply Simple string reply: OK.