Fix Config GET databases in cluster mode (#389)

* return correct db count when cluster is enabled

* expose ReadWriteSession flag

* create skeleton for CONFIG GET

* add CONFIG|GET tests

* fix formatting

libs/cluster/Server/ClusterConfig.cs

@@ -169,6 +169,16 @@ public bool IsKnown(string nodeid)
             return false;
         }
 
+        /// <summary>
+        /// Check if local node is a PRIMARY node
+        /// </summary>
+        public bool IsPrimary => LocalNodeRole == NodeRole.PRIMARY;
+
+        /// <summary>
+        /// Check if local node is a REPLICA node
+        /// </summary>
+        public bool IsReplica => LocalNodeRole == NodeRole.REPLICA;
+
         #region GetLocalNodeInfo
         /// <summary>
         /// Get local node ip

libs/cluster/Session/ClusterSession.cs

@@ -45,6 +45,8 @@ internal sealed unsafe partial class ClusterSession : IClusterSession
         /// </summary>
         bool readWriteSession = false;
 
+        public bool ReadWriteSession => clusterProvider.clusterManager.CurrentConfig.IsPrimary || readWriteSession;
+
         public void SetReadOnlySession() => readWriteSession = false;
         public void SetReadWriteSession() => readWriteSession = true;
 

libs/host/Configuration/Options.cs

@@ -629,8 +629,6 @@ private IAuthenticationSettings GetAuthenticationSettings(ILogger logger = null)
                     throw new Exception($"Authentication mode {AuthenticationMode} is not supported.");
             }
         }
-
-
     }
 
     /// <summary>

libs/server/Cluster/IClusterSession.cs

@@ -11,6 +11,11 @@ namespace Garnet.server
     /// </summary>
     public interface IClusterSession
     {
+        /// <summary>
+        /// Type of session
+        /// </summary>
+        bool ReadWriteSession { get; }
+
         /// <summary>
         /// Make this cluster session a read-only session
         /// </summary>

libs/server/Resp/AdminCommands.cs

@@ -96,30 +96,29 @@ private bool ProcessAdminCommands<TGarnetApi>(RespCommand command, ReadOnlySpan<
             }
             else if (command == RespCommand.CONFIG)
             {
-                if (!CheckACLAdminPermissions(bufSpan, count, out bool success))
+                if (!CheckACLAdminPermissions(bufSpan, count, out var success))
                 {
                     return success;
                 }
 
-                var param = GetCommand(bufSpan, out bool success1);
+                // Terminate early if command arguments are less than expected
+                if (count < 1)
+                {
+                    while (!RespWriteUtils.WriteBulkString("ERR wrong number of arguments for 'config' command"u8, ref dcurr, dend))
+                        SendAndReset();
+                    return true;
+                }
+
+                var param = GetCommand(bufSpan, out var success1);
                 if (!success1) return false;
+
                 if (param.SequenceEqual(CmdStrings.GET) || param.SequenceEqual(CmdStrings.get))
                 {
-                    if (count > 2)
-                    {
-                        if (!DrainCommands(bufSpan, count - 1))
-                            return false;
-                        errorFlag = true;
-                        errorCmd = Encoding.ASCII.GetString(param);
-                    }
-                    else
-                    {
-                        var key = GetCommand(bufSpan, out bool success2);
-                        if (!success2) return false;
+                    if (!NetworkConfigGet(bufSpan, count, out errorFlag))
+                        return false;
 
-                        while (!RespWriteUtils.WriteDirect(CmdStrings.GetConfig(key), ref dcurr, dend))
-                            SendAndReset();
-                    }
+                    if (errorFlag)
+                        errorCmd = Encoding.ASCII.GetString(CmdStrings.CONFIG) + "|" + Encoding.ASCII.GetString(param);
                 }
                 else if (param.SequenceEqual(CmdStrings.REWRITE) || param.SequenceEqual(CmdStrings.rewrite))
                 {

libs/server/Resp/CmdStrings.cs

@@ -10,16 +10,6 @@ namespace Garnet.server
     /// </summary>
     static partial class CmdStrings
     {
-        public static ReadOnlySpan<byte> GetConfig(ReadOnlySpan<byte> key)
-        {
-            if (key.SequenceEqual("timeout"u8)) return "*2\r\n$7\r\ntimeout\r\n$1\r\n0\r\n"u8;
-            else if (key.SequenceEqual("save"u8)) return "*2\r\n$4\r\nsave\r\n$0\r\n\r\n"u8;
-            else if (key.SequenceEqual("appendonly"u8)) return "*2\r\n$10\r\nappendonly\r\n$2\r\nno\r\n"u8;
-            else if (key.SequenceEqual("slave-read-only"u8)) return "$3\r\nyes\r\n"u8;
-            else if (key.SequenceEqual("databases"u8)) return "$2\r\n16\r\n"u8;
-            else return RESP_EMPTYLIST;
-        }
-
         /// <summary>
         /// Request strings
         /// </summary>

libs/server/Resp/RespServerSession.cs

@@ -654,15 +654,15 @@ private bool ProcessOtherCommands<TGarnetApi>(RespCommand command, byte subcmd,
 
         bool DrainCommands(ReadOnlySpan<byte> bufSpan, int count)
         {
-            for (int i = 0; i < count; i++)
+            for (var i = 0; i < count; i++)
             {
                 GetCommand(bufSpan, out bool success1);
                 if (!success1) return false;
             }
             return true;
         }
 
-        ReadOnlySpan<byte> GetCommand(ReadOnlySpan<byte> bufSpan, out bool success)
+        Span<byte> GetCommand(ReadOnlySpan<byte> bufSpan, out bool success)
         {
             var ptr = recvBufferPtr + readHead;
             var end = recvBufferPtr + bytesRead;
@@ -689,7 +689,7 @@ ReadOnlySpan<byte> GetCommand(ReadOnlySpan<byte> bufSpan, out bool success)
                 RespParsingException.ThrowUnexpectedToken(*ptr);
             }
 
-            var result = bufSpan.Slice(readHead, length);
+            var result = new Span<byte>(recvBufferPtr + readHead, length);
             readHead += length + 2;
             success = true;
 

libs/server/ServerConfig.cs

@@ -0,0 +1,107 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using Garnet.common;
+
+namespace Garnet.server
+{
+    static class ServerConfig
+    {
+        public static readonly HashSet<ServerConfigType> DefaultConfigType = Enum.GetValues<ServerConfigType>().
+            Where(e => e switch
+            {
+                ServerConfigType.NONE => false,
+                ServerConfigType.ALL => false,
+                _ => true
+            }).ToHashSet();
+
+        public static unsafe ServerConfigType GetConfig(Span<byte> parameter)
+        {
+            ConvertUtils.MakeUpperCase(parameter);
+            return parameter switch
+            {
+                _ when parameter.SequenceEqual("TIMEOUT"u8) => ServerConfigType.TIMEOUT,
+                _ when parameter.SequenceEqual("SAVE"u8) => ServerConfigType.SAVE,
+                _ when parameter.SequenceEqual("APPENDONLY"u8) => ServerConfigType.APPENDONLY,
+                _ when parameter.SequenceEqual("SLAVE-READ-ONLY"u8) => ServerConfigType.SLAVE_READ_ONLY,
+                _ when parameter.SequenceEqual("DATABASES"u8) => ServerConfigType.DATABASES,
+                _ when parameter.SequenceEqual("CLUSTER-NODE-TIMEOUT"u8) => ServerConfigType.CLUSTER_NODE_TIMEOUT,
+                _ when parameter.SequenceEqual("*"u8) => ServerConfigType.ALL,
+                _ => ServerConfigType.NONE,
+            };
+        }
+    }
+
+    internal sealed unsafe partial class RespServerSession : ServerSessionBase
+    {
+        private bool NetworkConfigGet(ReadOnlySpan<byte> bufSpan, int count, out bool errorFlag)
+        {
+            errorFlag = false;
+            if (count < 2)
+            {
+                if (!DrainCommands(bufSpan, count - 1))
+                    return false;
+                errorFlag = true;
+                return true;
+            }
+
+            // Extract requested parameters
+            HashSet<ServerConfigType> parameters = [];
+            var returnAll = false;
+            for (var i = 0; i < count - 1; i++)
+            {
+                var parameter = GetCommand(bufSpan, out bool success2);
+                if (!success2) return false;
+                var serverConfigType = ServerConfig.GetConfig(parameter);
+
+                if (returnAll) continue;
+                if (serverConfigType == ServerConfigType.ALL)
+                {
+                    parameters = ServerConfig.DefaultConfigType;
+                    returnAll = true;
+                    continue;
+                }
+
+                if (serverConfigType != ServerConfigType.NONE)
+                    _ = parameters.Add(serverConfigType);
+            }
+
+            // Generate response for matching parameters
+            if (parameters.Count > 0)
+            {
+                while (!RespWriteUtils.WriteArrayLength(parameters.Count * 2, ref dcurr, dend))
+                    SendAndReset();
+
+                foreach (var parameter in parameters)
+                {
+                    var parameterValue = parameter switch
+                    {
+                        ServerConfigType.TIMEOUT => "$7\r\ntimeout\r\n$1\r\n0\r\n"u8,
+                        ServerConfigType.SAVE => "$4\r\nsave\r\n$0\r\n\r\n"u8,
+                        ServerConfigType.APPENDONLY => storeWrapper.serverOptions.EnableAOF ? "$10\r\nappendonly\r\n$3\r\nyes\r\n"u8 : "$10\r\nappendonly\r\n$2\r\nno\r\n"u8,
+                        ServerConfigType.SLAVE_READ_ONLY => clusterSession == null || clusterSession.ReadWriteSession ? "$15\r\nslave-read-only\r\n$2\r\nno\r\n"u8 : "$15\r\nslave-read-only\r\n$3\r\nyes\r\n"u8,
+                        ServerConfigType.DATABASES => storeWrapper.serverOptions.EnableCluster ? "$9\r\ndatabases\r\n$1\r\n1\r\n"u8 : "$9\r\ndatabases\r\n$2\r\n16\r\n"u8,
+                        ServerConfigType.CLUSTER_NODE_TIMEOUT => Encoding.ASCII.GetBytes($"$20\r\ncluster-node-timeout\r\n${storeWrapper.serverOptions.ClusterTimeout.ToString().Length}\r\n{storeWrapper.serverOptions.ClusterTimeout}\r\n"),
+                        ServerConfigType.NONE => throw new NotImplementedException(),
+                        ServerConfigType.ALL => throw new NotImplementedException(),
+                        _ => throw new NotImplementedException()
+                    };
+
+                    while (!RespWriteUtils.WriteDirect(parameterValue, ref dcurr, dend))
+                        SendAndReset();
+                }
+            }
+            else
+            {
+                while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_EMPTYLIST, ref dcurr, dend))
+                    SendAndReset();
+            }
+
+            return true;
+        }
+    }
+}

libs/server/ServerConfigType.cs

@@ -0,0 +1,17 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+namespace Garnet.server
+{
+    internal enum ServerConfigType : int
+    {
+        NONE,
+        ALL,
+        TIMEOUT,
+        SAVE,
+        APPENDONLY,
+        SLAVE_READ_ONLY,
+        DATABASES,
+        CLUSTER_NODE_TIMEOUT
+    }
+}

test/Garnet.test/RespAdminCommandsTests.cs

@@ -487,6 +487,57 @@ public async Task SeFlushDBTest([Values] bool async, [Values] bool unsafetruncat
             Assert.IsTrue(_value.IsNull);
         }
 
+        [Test]
+        [TestCase("timeout", "0")]
+        [TestCase("save", "")]
+        [TestCase("appendonly", "no")]
+        [TestCase("slave-read-only", "no")]
+        [TestCase("databases", "16")]
+        [TestCase("cluster-node-timeout", "60")]
+        public void SimpleConfigGet(string parameter, string parameterValue)
+        {
+            using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig());
+            var db = redis.GetDatabase(0);
+
+            var result = (string[])db.Execute("CONFIG", "GET", parameter);
+
+            Assert.AreEqual(parameter, result[0]);
+            Assert.AreEqual(parameterValue, result[1]);
+        }
+
+        #endregion
+
+        #region NegativeTests
+
+        [Test]
+        public void ConfigWrongNumberOfArguments()
+        {
+            using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig());
+            var db = redis.GetDatabase(0);
+            try
+            {
+                db.Execute("CONFIG");
+            }
+            catch (Exception ex)
+            {
+                Assert.AreEqual("ERR wrong number of arguments for 'CONFIG' command", ex.Message);
+            }
+        }
+
+        [Test]
+        public void ConfigGetWrongNumberOfArguments()
+        {
+            using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig());
+            var db = redis.GetDatabase(0);
+            try
+            {
+                db.Execute("CONFIG", "GET");
+            }
+            catch (Exception ex)
+            {
+                Assert.AreEqual("ERR wrong number of arguments for 'CONFIG|GET' command", ex.Message);
+            }
+        }
         #endregion
     }
 }