-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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.
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
HDDS-2169. Avoid buffer copies while submitting client requests in Ratis #1517
Conversation
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
/retest |
Thanks @szetszwo for working on this. With the patch, while running the tests in TestDataValidateWithUnsafeByteOperations, the below issue is observed.
We need to change to code in ContainerStateMachine to parse the data correctly in writeStateMachineData. Can you please check? |
💔 -1 overall
This message was automatically generated. |
Thank @bshashikant for catching the bug. The ByteString from LogEntryProto should remain unchanged. Only the message from client requests need to be changed. Just have addressed this bug in commit e6b25d101818031b64e61764aa6ce712250a1541 (HEAD -> trunk, origin/trunk, origin/HEAD)
|
💔 -1 overall
This message was automatically generated. |
/retest |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Thanks @szetszwo for updating the patch. i think We need to change the parsing logic applyTransaction call in ContainerStateMachine as well because the integration test failures are related here: |
Oops, need one more fix (a4ae0852f818f6a49403b784790b1babb3df71f5) as shown below since it calls request.getMessage().getContent() again in startTransaction(..). Thanks @bshashikant.
|
💔 -1 overall
This message was automatically generated. |
Thanks @szetszwo for updating the patch. I tried to run the tests in TestDataValidateWithUnsafeByteOperations and i see the following exception being thrown: Can you please check? |
Thanks @bshashikant. That is a bug in ChunkUtils
It should call data.remaining() instead of data.capacity(). |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
…tis. Contributed by Tsz-wo Sze(#1517).
Thanks @szetszwo for working on this. I have added the relavent change below as well while submitting the close container request.
` |
Thanks @bshashikant ! |
…tis. Contributed by Tsz-wo Sze(apache#1517).
…tis. Contributed by Tsz-wo Sze(apache#1517).
https://issues.apache.org/jira/browse/HDDS-2169