Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Parsing Regression in GarnetClient and GarnetClientSession #456

Merged
merged 15 commits into from
Jun 12, 2024
Merged

Fix Parsing Regression in GarnetClient and GarnetClientSession #456

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
9f01d7a
fix regression in GarnetClientSession process responses to allow null…
vazois Jun 9, 2024
0d91639
propagate more allow nulls in GarnetClient and GarnetClientSession
vazois Jun 9, 2024
f0da989
fix gcs breaking change
vazois Jun 10, 2024
54d0bf4
cleanup online GarnetClientSession
vazois Jun 10, 2024
f829ad7
more cleanup
vazois Jun 10, 2024
892a13f
remove isArray parameter where not needed
vazois Jun 10, 2024
6423516
remove allowNulls
vazois Jun 10, 2024
8841c11
fix build issue
vazois Jun 10, 2024
7ae1447
introduced descriptive name
vazois Jun 10, 2024
f3a575a
add retry logic
vazois Jun 10, 2024
dd6c766
add RespReadResponseUtils wrapper for Resp paring at client side
vazois Jun 11, 2024
17c49ec
address review comments
vazois Jun 11, 2024
c59211b
separate ReadArrayLenght into ReadSignedArrayLength and ReadUnsignedA…
vazois Jun 11, 2024
d38ccb2
add some comments
vazois Jun 11, 2024
6e8449f
eliminate torrent of logging messages in RespOnlineBench
vazois Jun 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion benchmark/BDN.benchmark/Resp/RespIntegerReadBenchmarks.cs
View file
Open in desktop
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
Open in desktop
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
792 changes: 384 additions & 408 deletions benchmark/Resp.benchmark/RespOnlineBench.cs
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion benchmark/Resp.benchmark/RespPerfBench.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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