You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "ivandika3 (via GitHub)" <gi...@apache.org> on 2024/03/25 10:42:26 UTC
[PR] HDDS-10587. Reset the thread-local MessageDigest instance during exception [ozone]
ivandika3 opened a new pull request, #6435:
URL: https://github.com/apache/ozone/pull/6435
## What changes were proposed in this pull request?
Currently, the MessageDigest instance is a thread local variable (one per S3G Jetty thread). MessageDigest requires the call to either MessageDigest#digest or MessageDigest#reset to reset the digest.
In normal ObjectEndpoint#put flow, MessageDigest#digest is called after the data has been written to the datanodes, before the key is committed. However, if an IOException happens (e.g. EOFException due to client cancelling during the write), the digest will not be reset and remains in the inconsistent state. This will affect the subsequent request that uses the same thread and therefore the ETag generated will be completely different from the md5 hash of the object causing AWS S3 SDK to detect inconsistent hash when downloading the object.
The issue can be replicated using an S3G with a single thread and doing three put-object operations for the same key and same payload. You can set the `hadoop.http.max.threads` in `ozone-site.xml` to a small value (e.g. 4) to increase the chance of the same thread handling the request.
- 1st put-object: cancel the operation before it put-object operation can finish, ensure the EOFException is thrown in the S3Gateway logs
- 2nd put-object: let the put-object finish. The resulting ETag will not be the same as the md5 digest of the payload (you might need to do this for a few time since the S3G thread might not be the same from the previous call)
- 3rd put-object: also let the put-object finish. Since the previous put-object reset the digest, the resulting ETag will be correct.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10587
## How was this patch tested?
Manual test from Ozone Intellij IDE setup as shown in the description.
Ref: https://cwiki.apache.org/confluence/display/OZONE/Run+Ozone+cluster+from+IDE
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10587. Reset the thread-local MessageDigest instance during exception [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #6435:
URL: https://github.com/apache/ozone/pull/6435#issuecomment-2018902669
@vtutrinov @myskov Could you help take a look when you have time?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10587. Reset the thread-local MessageDigest instance during exception [ozone]
Posted by "vtutrinov (via GitHub)" <gi...@apache.org>.
vtutrinov commented on PR #6435:
URL: https://github.com/apache/ozone/pull/6435#issuecomment-2020127127
@ivandika3 thanks for the PR and the unit tests, LGTM, +1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10587. Reset the thread-local MessageDigest instance during exception [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #6435:
URL: https://github.com/apache/ozone/pull/6435
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10587. Reset the thread-local MessageDigest instance during exception [ozone]
Posted by "vtutrinov (via GitHub)" <gi...@apache.org>.
vtutrinov commented on PR #6435:
URL: https://github.com/apache/ozone/pull/6435#issuecomment-2019716052
@ivandika3 I'd like to see a set of unit tests to check that the digest message will be resetted in case of exception. An example is:
```java
ObjectEndpoint endpoint = spy(...);
try (MockedStatic staticMock = mockStatic(ObjectEndpointStreaming.class)) {
staticMock.when(() -> ObjectEndpointStreaming.put(...)).thenThrow(IOException.class);
endpoint.put(...);
verify(endpoint.getETagProvider()).reset();
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10587. Reset the thread-local MessageDigest instance during exception [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6435:
URL: https://github.com/apache/ozone/pull/6435#issuecomment-2021077988
Thanks @ivandika3 for the patch, @vtutrinov for the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10587. Reset the thread-local MessageDigest instance during exception [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #6435:
URL: https://github.com/apache/ozone/pull/6435#issuecomment-2020054728
@vtutrinov Thank you for the review and unit test idea. I have added unit tests for put, copy, and MPU part upload cases. PTAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10587. Reset the thread-local MessageDigest instance during exception [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #6435:
URL: https://github.com/apache/ozone/pull/6435#issuecomment-2021745197
Thanks @vtutrinov for the review and @adoroszlai for the merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org