[DEV] Verbatim String Fixes (#1777)

* fix verbatim string uses to handle 'too large for send buffer' cases; more general fix is needed throughout, but this should fix INFO with large numbers of clients or nodes

* address feedback

* address feedback; rollback unrelated change, simplify WriteLargeVerbatimString

* nit

---------

Co-authored-by: Badrish Chandramouli <badrishc@microsoft.com>

Author: kevin-montrose

libs/common/RespWriteUtils.cs

@@ -761,6 +761,34 @@ public static bool TryWriteVerbatimString(ReadOnlySpan<byte> str, ReadOnlySpan<b
             return true;
         }
 
+        /// <summary>
+        /// Write verbtatim string header.
+        /// 
+        /// That is ={len}\r\n{ext}:
+        /// </summary>
+        public static bool TryWriteVerbatimStringHeader(ReadOnlySpan<byte> str, ReadOnlySpan<byte> ext, ref byte* curr, byte* end)
+        {
+            Debug.Assert(ext.Length == 3);
+
+            // Verbatim string length includes the type metadata.
+            // So ext (3 bytes) + ':' (1 byte separator) + str
+            var actualLength = 3 + 1 + str.Length;
+            var itemDigits = NumUtils.CountDigits(actualLength);
+
+            var headerLen = 1 + itemDigits + 2 + 3 + 1;
+            if (headerLen > (int)(end - curr))
+                return false;
+
+            *curr++ = (byte)'=';
+            NumUtils.WriteInt32(actualLength, itemDigits, ref curr);
+            WriteNewline(ref curr);
+            ext.CopyTo(new Span<byte>(curr, 3));
+            curr += 3;
+            *curr++ = (byte)':';
+
+            return true;
+        }
+
         /// <summary>
         /// Write RESP3 true
         /// </summary>

libs/server/Metrics/Info/InfoCommand.cs

@@ -71,7 +71,7 @@ private bool NetworkINFO()
                 var info = garnetInfo.GetRespInfo(sectionsArr, activeDbId, storeWrapper);
                 if (!string.IsNullOrEmpty(info))
                 {
-                    WriteVerbatimString(Encoding.ASCII.GetBytes(info));
+                    WriteLargeVerbatimString(Encoding.ASCII.GetBytes(info));
                 }
                 else
                 {

libs/server/Resp/ClientCommands.cs

@@ -160,7 +160,7 @@ private bool NetworkCLIENTLIST()
 
                     resultSb.Append("\n");
                     var result = resultSb.ToString();
-                    WriteVerbatimString(Encoding.ASCII.GetBytes(result));
+                    WriteLargeVerbatimString(Encoding.ASCII.GetBytes(result));
 
                     return true;
                 }
@@ -194,7 +194,7 @@ private bool NetworkCLIENTINFO()
 
             resultSb.Append("\n");
             var result = resultSb.ToString();
-            WriteVerbatimString(Encoding.ASCII.GetBytes(result));
+            WriteLargeVerbatimString(Encoding.ASCII.GetBytes(result));
 
             return true;
         }

libs/server/Resp/RespServerSessionOutput.cs

@@ -272,17 +272,25 @@ private void WriteUtf8BulkString(ReadOnlySpan<char> chars)
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private void WriteVerbatimString(scoped ReadOnlySpan<byte> item, scoped ReadOnlySpan<byte> ext = default)
+        private void WriteLargeVerbatimString(scoped ReadOnlySpan<byte> item, scoped ReadOnlySpan<byte> ext = default)
         {
             if (respProtocolVersion >= 3)
             {
-                while (!RespWriteUtils.TryWriteVerbatimString(item, ext.IsEmpty ? RespStrings.VerbatimTxt : ext, ref dcurr, dend))
+                ext = ext.IsEmpty ? RespStrings.VerbatimTxt : ext;
+
+                while (!RespWriteUtils.TryWriteVerbatimStringHeader(item, ext, ref dcurr, dend))
+                    SendAndReset();
+
+                // Write out item
+                WriteDirectLarge(item);
+
+                while (!RespWriteUtils.TryWriteNewLine(ref dcurr, dend))
                     SendAndReset();
             }
             else
             {
-                while (!RespWriteUtils.TryWriteBulkString(item, ref dcurr, dend))
-                    SendAndReset();
+                // RESP2 just gets a bulk string
+                WriteDirectLargeRespString(item);
             }
         }
     }

test/Garnet.test/RespInfoTests.cs

@@ -34,11 +34,12 @@ public void TearDown()
             TestUtils.OnTearDown();
         }
 
-        [Test]
-        public void ResetStatsTest()
+        [TestCase(RedisProtocol.Resp2)]
+        [TestCase(RedisProtocol.Resp3)]
+        public void ResetStatsTest(RedisProtocol protocol)
         {
             TimeSpan metricsUpdateDelay = TimeSpan.FromSeconds(1.1);
-            using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig());
+            using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig(protocol: protocol));
             var db = redis.GetDatabase(0);
 
             var infoResult = db.Execute("INFO").ToString();
@@ -93,13 +94,15 @@ static long ParseUptime(string info) =>
             ClassicAssert.Greater(second, first, "uptime_in_seconds should increase between INFO calls");
         }
 
-        [Test]
-        [TestCase("ALL")]
-        [TestCase("DEFAULT")]
-        [TestCase("EVERYTHING")]
-        public void InfoSectionOptionsTest(string option)
+        [TestCase("ALL", RedisProtocol.Resp2)]
+        [TestCase("ALL", RedisProtocol.Resp3)]
+        [TestCase("DEFAULT", RedisProtocol.Resp2)]
+        [TestCase("DEFAULT", RedisProtocol.Resp3)]
+        [TestCase("EVERYTHING", RedisProtocol.Resp2)]
+        [TestCase("EVERYTHING", RedisProtocol.Resp3)]
+        public void InfoSectionOptionsTest(string option, RedisProtocol protocol)
         {
-            using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig());
+            using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig(protocol: protocol));
             var db = redis.GetDatabase(0);
 
             var infoResult = db.Execute("INFO", option).ToString();
@@ -132,10 +135,11 @@ public void InfoSectionOptionsTest(string option)
             ClassicAssert.IsFalse(infoResult.Contains("# Commandstats"), $"INFO {option} should not contain Commandstats section");
         }
 
-        [Test]
-        public void InfoDefaultMatchesNoArgsTest()
+        [TestCase(RedisProtocol.Resp2)]
+        [TestCase(RedisProtocol.Resp3)]
+        public void InfoDefaultMatchesNoArgsTest(RedisProtocol protocol)
         {
-            using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig());
+            using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig(protocol: protocol));
             var db = redis.GetDatabase(0);
 
             var infoNoArgs = db.Execute("INFO").ToString();