Skip to content

Commit

Permalink
Fix Parsing Regression in GarnetClient and GarnetClientSession (micro…
Browse files Browse the repository at this point in the history 
…soft#456)

* fix regression in GarnetClientSession process responses to allow null values

* propagate more allow nulls in GarnetClient and GarnetClientSession

* fix gcs breaking change

* cleanup online GarnetClientSession

* more cleanup

* remove isArray parameter where not needed

* remove allowNulls

* fix build issue

* introduced descriptive name

* add retry logic

* add RespReadResponseUtils wrapper for Resp paring at client side

* address review comments

* separate ReadArrayLenght into ReadSignedArrayLength and ReadUnsignedArrayLength

* add some comments

* eliminate torrent of logging messages in RespOnlineBench
  • Loading branch information
@vazois
vazois authored Jun 12, 2024
1 parent 91544e1 commit 2647813
Show file tree
Hide file tree
Showing 17 changed files with 820 additions and 681 deletions.
2 changes: 1 addition & 1 deletion benchmark/BDN.benchmark/Resp/RespIntegerReadBenchmarks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public int ReadLengthHeader(AsciiTestCase testCase)
fixed (byte* inputPtr = testCase.Bytes)
{
var start = inputPtr;
RespReadUtils.ReadLengthHeader(out var value, ref start, start + testCase.Bytes.Length, allowNull: true);
RespReadUtils.ReadSignedLengthHeader(out var value, ref start, start + testCase.Bytes.Length);
return value;
}
}
Expand Down
2 changes: 1 addition & 1 deletion benchmark/Resp.benchmark/ReqGen.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ private void ProcessArgs(int i, byte[] buffer)
fixed (byte* buf = buffer)
{
byte* ptr = buf;
RespReadUtils.ReadArrayLength(out int count, ref ptr, buf + buffer.Length);
RespReadUtils.ReadUnsignedArrayLength(out int count, ref ptr, buf + buffer.Length);
RespReadUtils.ReadStringWithLengthHeader(out var cmd, ref ptr, buf + buffer.Length);

for (int j = 0; j < count - 1; j++)
Expand Down
821 changes: 408 additions & 413 deletions benchmark/Resp.benchmark/RespOnlineBench.cs
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion benchmark/Resp.benchmark/RespPerfBench.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ private unsafe void GetDBSIZE(int loadDbThreads)
fixed (byte* buf = client.ResponseBuffer)
{
byte* ptr = buf;
RespReadUtils.ReadIntegerAsString(out dbSize, ref ptr, ptr + client.ResponseBuffer.Length);
RespReadResponseUtils.ReadIntegerAsString(out dbSize, ref ptr, ptr + client.ResponseBuffer.Length);
}
}
else
Expand Down
10 changes: 5 additions & 5 deletions libs/client/ClientSession/GarnetClientSession.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,28 +340,28 @@ private int ProcessReplies(byte* recvBufferPtr, int bytesRead)
switch (*ptr)
{
case (byte)'+':
if (!RespReadUtils.ReadSimpleString(out result, ref ptr, recvBufferPtr + bytesRead))
if (!RespReadResponseUtils.ReadSimpleString(out result, ref ptr, recvBufferPtr + bytesRead))
success = false;
break;
case (byte)':':
if (!RespReadUtils.ReadIntegerAsString(out result, ref ptr, recvBufferPtr + bytesRead))
if (!RespReadResponseUtils.ReadIntegerAsString(out result, ref ptr, recvBufferPtr + bytesRead))
success = false;
break;

case (byte)'-':
error = true;
if (!RespReadUtils.ReadErrorAsString(out result, ref ptr, recvBufferPtr + bytesRead))
if (!RespReadResponseUtils.ReadErrorAsString(out result, ref ptr, recvBufferPtr + bytesRead))
success = false;
break;

case (byte)'$':
if (!RespReadUtils.ReadStringWithLengthHeader(out result, ref ptr, recvBufferPtr + bytesRead))
if (!RespReadResponseUtils.ReadStringWithLengthHeader(out result, ref ptr, recvBufferPtr + bytesRead))
success = false;
break;

case (byte)'*':
isArray = true;
if (!RespReadUtils.ReadStringArrayWithLengthHeader(out resultArray, ref ptr, recvBufferPtr + bytesRead))
if (!RespReadResponseUtils.ReadStringArrayWithLengthHeader(out resultArray, ref ptr, recvBufferPtr + bytesRead))
success = false;
break;

Expand Down
47 changes: 23 additions & 24 deletions libs/client/GarnetClientProcessReplies.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,27 +31,27 @@ unsafe bool ProcessReplyAsString(ref byte* ptr, byte* end, out string result, ou
result = "OK";
break;
}
if (!RespReadUtils.ReadSimpleString(out result, ref ptr, end))
if (!RespReadResponseUtils.ReadSimpleString(out result, ref ptr, end))
return false;
break;

case (byte)':':
if (!RespReadUtils.ReadIntegerAsString(out result, ref ptr, end))
if (!RespReadResponseUtils.ReadIntegerAsString(out result, ref ptr, end))
return false;
break;

case (byte)'-':
if (!RespReadUtils.ReadErrorAsString(out error, ref ptr, end))
if (!RespReadResponseUtils.ReadErrorAsString(out error, ref ptr, end))
return false;
break;

case (byte)'$':
if (!RespReadUtils.ReadStringWithLengthHeader(out result, ref ptr, end, allowNull: true))
if (!RespReadResponseUtils.ReadStringWithLengthHeader(out result, ref ptr, end))
return false;
break;

case (byte)'*':
if (!RespReadUtils.ReadStringArrayWithLengthHeader(out var resultArray, ref ptr, end))
if (!RespReadResponseUtils.ReadStringArrayWithLengthHeader(out var resultArray, ref ptr, end))
return false;
// Return first element of array
result = resultArray[0];
Expand All @@ -78,12 +78,12 @@ unsafe bool ProcessReplyAsNumber(ref byte* ptr, byte* end, out long number, out
break;

case (byte)'-':
if (!RespReadUtils.ReadErrorAsString(out error, ref ptr, end))
if (!RespReadResponseUtils.ReadErrorAsString(out error, ref ptr, end))
return false;
break;

case (byte)'$':
if (!RespReadUtils.ReadIntWithLengthHeader(out var intWithLength, ref ptr, end))
if (!RespReadResponseUtils.ReadIntWithLengthHeader(out var intWithLength, ref ptr, end))
return false;
number = intWithLength;
break;
Expand All @@ -95,7 +95,6 @@ unsafe bool ProcessReplyAsNumber(ref byte* ptr, byte* end, out long number, out
return true;
}


unsafe bool ProcessReplyAsStringArray(ref byte* ptr, byte* end, out string[] result, out string error)
{
result = null;
Expand All @@ -108,32 +107,32 @@ unsafe bool ProcessReplyAsStringArray(ref byte* ptr, byte* end, out string[] res
if ((ptr + 5 <= end) && (*(int*)(ptr + 1) == 168643407))
{
ptr += 5;
result = new[] { "OK" };
result = ["OK"];
break;
}
if (!RespReadUtils.ReadSimpleString(out var _result, ref ptr, end))
if (!RespReadResponseUtils.ReadSimpleString(out var _result, ref ptr, end))
return false;
result = new[] { _result };
result = [_result];
break;
case (byte)':':
if (!RespReadUtils.ReadIntegerAsString(out _result, ref ptr, end))
if (!RespReadResponseUtils.ReadIntegerAsString(out _result, ref ptr, end))
return false;
result = new[] { _result };
result = [_result];
break;

case (byte)'-':
if (!RespReadUtils.ReadErrorAsString(out error, ref ptr, end))
if (!RespReadResponseUtils.ReadErrorAsString(out error, ref ptr, end))
return false;
break;

case (byte)'$':
if (!RespReadUtils.ReadStringWithLengthHeader(out _result, ref ptr, end))
if (!RespReadResponseUtils.ReadStringWithLengthHeader(out _result, ref ptr, end))
return false;
result = new[] { _result };
result = [_result];
break;

case (byte)'*':
if (!RespReadUtils.ReadStringArrayWithLengthHeader(out result, ref ptr, end))
if (!RespReadResponseUtils.ReadStringArrayWithLengthHeader(out result, ref ptr, end))
return false;
break;

Expand All @@ -160,29 +159,29 @@ unsafe bool ProcessReplyAsMemoryByte(ref byte* ptr, byte* end, out MemoryResult<
result = RESP_OK;
break;
}
if (!RespReadUtils.ReadSimpleString(memoryPool, out result, ref ptr, end))
if (!RespReadResponseUtils.ReadSimpleString(memoryPool, out result, ref ptr, end))
return false;
break;
case (byte)':':
if (!RespReadUtils.ReadIntegerAsString(memoryPool, out result, ref ptr, end))
if (!RespReadResponseUtils.ReadIntegerAsString(memoryPool, out result, ref ptr, end))
return false;
break;

case (byte)'-':
if (!RespReadUtils.ReadErrorAsString(out error, ref ptr, end))
if (!RespReadResponseUtils.ReadErrorAsString(out error, ref ptr, end))
return false;
break;

case (byte)'$':
if (!RespReadUtils.ReadStringWithLengthHeader(memoryPool, out result, ref ptr, end))
if (!RespReadResponseUtils.ReadStringWithLengthHeader(memoryPool, out result, ref ptr, end))
return false;
break;

case (byte)'*':
if (!RespReadUtils.ReadStringArrayWithLengthHeader(memoryPool, out var resultArray, ref ptr, end))
if (!RespReadResponseUtils.ReadStringArrayWithLengthHeader(memoryPool, out var resultArray, ref ptr, end))
return false;
// Return first element of array
for (int i = 1; i < resultArray.Length; i++)
for (var i = 1; i < resultArray.Length; i++)
resultArray[i].Dispose();
result = resultArray[0];
break;
Expand All @@ -202,7 +201,7 @@ unsafe bool ProcessReplyAsMemoryByteArray(ref byte* ptr, byte* end, out MemoryRe
switch (*ptr)
{
case (byte)'*':
if (!RespReadUtils.ReadStringArrayWithLengthHeader(memoryPool, out var resultArray, ref ptr, end))
if (!RespReadResponseUtils.ReadStringArrayWithLengthHeader(memoryPool, out var resultArray, ref ptr, end))
return false;
result = resultArray;
break;
Expand Down
Loading

0 comments on commit 2647813

Please sign in to comment.