You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/12/06 04:44:49 UTC
[GitHub] [ozone] k5342 opened a new pull request, #4048: HDDS-7596. Fix OM crash due to a corner case for FSO-enabled bucket
k5342 opened a new pull request, #4048:
URL: https://github.com/apache/ozone/pull/4048
## What changes were proposed in this pull request?
When `keyToDelete.getDataSize()` returns zero and a request for FSO-enable bucket, a `CompleteMultipartUpload` request will crash with NullPointerException by attempting to read a bucketInfo. This patch proposes to prevent crash that changes not to read bucketInfo in the request handler.
```
2022-12-02 20:48:24,809 [OMDoubleBufferFlushThread] ERROR org.apache.hadoop.ozone.om.ratis.OzoneManagerDoubleBuffer: Terminating with exit status 2: OMDoubleBuffer flush thread OMDoubleBufferFlushThread encountered Throwable error
java.lang.NullPointerException: Cannot invoke "org.apache.hadoop.ozone.om.helpers.OmBucketInfo.getObjectID()" because the return value of "org.apache.hadoop.ozone.om.response.s3.multipart.S3MultipartUploadCompleteResponseWithFSO.getOmBucketInfo()" is null
at org.apache.hadoop.ozone.om.response.s3.multipart.S3MultipartUploadCompleteResponseWithFSO.addToKeyTable(S3MultipartUploadCompleteResponseWithFSO.java:90)
at org.apache.hadoop.ozone.om.response.s3.multipart.S3MultipartUploadCompleteResponse.addToDBBatch(S3MultipartUploadCompleteResponse.java:101)
at org.apache.hadoop.ozone.om.response.OMClientResponse.checkAndUpdateDB(OMClientResponse.java:73)
at org.apache.hadoop.ozone.om.ratis.OzoneManagerDoubleBuffer.lambda$2(OzoneManagerDoubleBuffer.java:283)
at org.apache.hadoop.ozone.om.ratis.OzoneManagerDoubleBuffer.addToBatchWithTrace(OzoneManagerDoubleBuffer.java:228)
at org.apache.hadoop.ozone.om.ratis.OzoneManagerDoubleBuffer.lambda$1(OzoneManagerDoubleBuffer.java:281)
at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133)
at org.apache.hadoop.ozone.om.ratis.OzoneManagerDoubleBuffer.flushTransactions(OzoneManagerDoubleBuffer.java:277)
at java.base/java.lang.Thread.run(Thread.java:833)
```
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7596
## How was this patch tested?
- checked manually in our environment
- fix annotations to CheckForNull
--
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
[GitHub] [ozone] adoroszlai commented on pull request #4048: HDDS-7596. Fix OM crash due to a corner case for FSO-enabled bucket
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4048:
URL: https://github.com/apache/ozone/pull/4048#issuecomment-1435714299
Thanks @k5342 for the fix, @DaveTeng0, @neils-dev, @sumitagrawl 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
[GitHub] [ozone] k5342 commented on pull request #4048: HDDS-7596. Fix OM crash due to a corner case for FSO-enabled bucket
Posted by "k5342 (via GitHub)" <gi...@apache.org>.
k5342 commented on PR #4048:
URL: https://github.com/apache/ozone/pull/4048#issuecomment-1423778370
Thank you for the clarification, @neils-dev. I've added a small test based on `testAddDBToBatch()`. CI is still running, but could you look at this if it passes? https://github.com/apache/ozone/pull/4048/commits/80cca80fc6b206a22b2ba75efb56c4004a0673f7
--
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
[GitHub] [ozone] adoroszlai merged pull request #4048: HDDS-7596. Fix OM crash due to a corner case for FSO-enabled bucket
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #4048:
URL: https://github.com/apache/ozone/pull/4048
--
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
[GitHub] [ozone] k5342 commented on a diff in pull request #4048: HDDS-7596. Fix OM crash due to a corner case for FSO-enabled bucket
Posted by "k5342 (via GitHub)" <gi...@apache.org>.
k5342 commented on code in PR #4048:
URL: https://github.com/apache/ozone/pull/4048#discussion_r1097021328
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadCompleteResponse.java:
##########
@@ -147,6 +148,7 @@ protected List<OmKeyInfo> getPartsUnusedList() {
return partsUnusedList;
}
+ @CheckForNull
Review Comment:
Sorry, I revised the commit for cleanup: https://github.com/apache/ozone/pull/4048/commits/ea2152696d9f928a57987ee57a8b1b7570c1894f; checked at my local and now checking with the CI...
--
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
[GitHub] [ozone] neils-dev commented on a diff in pull request #4048: HDDS-7596. Fix OM crash due to a corner case for FSO-enabled bucket
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on code in PR #4048:
URL: https://github.com/apache/ozone/pull/4048#discussion_r1100926990
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadCompleteResponse.java:
##########
@@ -147,6 +148,7 @@ protected List<OmKeyInfo> getPartsUnusedList() {
return partsUnusedList;
}
+ @CheckForNull
Review Comment:
looks good. Thanks for the cleanup.
--
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
[GitHub] [ozone] neils-dev commented on pull request #4048: HDDS-7596. Fix OM crash due to a corner case for FSO-enabled bucket
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on PR #4048:
URL: https://github.com/apache/ozone/pull/4048#issuecomment-1423577129
> What do you think about the granularity for testing?
Thanks @k5342 . A test like tests within `TestS3MultipartUploadCompleteResponseWithFSO.java` with
```
S3MultipartUploadCompleteResponse s3MultipartUploadCompleteResponse =
createS3CompleteMPUResponseFSO(volumeName, bucketName, parentID,
keyName, multipartUploadID, omKeyInfoFSO,
OzoneManagerProtocolProtos.Status.OK, unUsedParts,
omBucketInfo, null);
```
using null `omBucketInfo` triggering the `S3MultipartUploadCompleteResponse.addToDBBatch()` to call `S3MultipartUploadCompleteResponseWithFSO.addToKeyTable()`. Result is not raising an NPE exception. Similar to testing the conditions triggering the NPE described in this Jira and PR.
--
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
[GitHub] [ozone] DaveTeng0 commented on pull request #4048: HDDS-7596. Fix OM crash due to a corner case for FSO-enabled bucket
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on PR #4048:
URL: https://github.com/apache/ozone/pull/4048#issuecomment-1408262869
Looks good to me! Let's get this merged!
--
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
[GitHub] [ozone] k5342 commented on pull request #4048: HDDS-7596. Fix OM crash due to a corner case for FSO-enabled bucket
Posted by "k5342 (via GitHub)" <gi...@apache.org>.
k5342 commented on PR #4048:
URL: https://github.com/apache/ozone/pull/4048#issuecomment-1418894521
> A test for this corner case with a mulitpart request with the fso zero-size null bucketInfo is needed.
Thank you for your comments! Of course, I agree with you about adding some test cases.
A test case is meant to be a test for `getOmClientResponse()`, right? Ideally, I think it's better for test coverage if we can put a test for `validateAndUpdateCache()`, but this function is a bit complex for unit testing, and I'm not sure how it can be reproduced as a unit testing to reach both `keyToDelete != null` and `usedBytesDiff == 0` (finally call `omBucketInfo = null`) because I think this might be a bug from some race conditions. What do you think about the granularity for testing?
--
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
[GitHub] [ozone] k5342 commented on a diff in pull request #4048: HDDS-7596. Fix OM crash due to a corner case for FSO-enabled bucket
Posted by "k5342 (via GitHub)" <gi...@apache.org>.
k5342 commented on code in PR #4048:
URL: https://github.com/apache/ozone/pull/4048#discussion_r1097009667
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadCompleteResponse.java:
##########
@@ -147,6 +148,7 @@ protected List<OmKeyInfo> getPartsUnusedList() {
return partsUnusedList;
}
+ @CheckForNull
Review Comment:
`getOmBucketInfo()`, `getPartsUnusedList()` and `getMultipartKey()` are unused. I've done with this commit: ~~https://github.com/apache/ozone/pull/4048/commits/955ed39197104e174a7971ff609e7b609d6c162d~~ https://github.com/apache/ozone/pull/4048/commits/ea2152696d9f928a57987ee57a8b1b7570c1894f
--
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
[GitHub] [ozone] neils-dev commented on pull request #4048: HDDS-7596. Fix OM crash due to a corner case for FSO-enabled bucket
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on PR #4048:
URL: https://github.com/apache/ozone/pull/4048#issuecomment-1416621944
Thanks @k5342 for filing the jira for this issue and providing a patch! Ditto,
> @k5342 could you also add a test case for this.
A test for this corner case with a mulitpart request with the fso zero-size null `bucketInfo` is needed.
--
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
[GitHub] [ozone] neils-dev commented on a diff in pull request #4048: HDDS-7596. Fix OM crash due to a corner case for FSO-enabled bucket
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on code in PR #4048:
URL: https://github.com/apache/ozone/pull/4048#discussion_r1096476624
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadCompleteResponse.java:
##########
@@ -147,6 +148,7 @@ protected List<OmKeyInfo> getPartsUnusedList() {
return partsUnusedList;
}
+ @CheckForNull
Review Comment:
the public method, `getOmBucketInfo()` is unnecessary with this change. Now it is no longer used from the `S3MultipartUploadCompleteResponseWithFSO`, as we just use the `bucketID`. The method can be removed.
Also please check if `S3MultipartUploadCompleteResponse.getPartsUnusedList `method is unused. If so, it to can be removed.
https://github.com/apache/ozone/blob/037492f839903c67709d44bd5a4ea9ca9ea093a9/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadCompleteResponse.java#L146
--
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
[GitHub] [ozone] kerneltime commented on pull request #4048: HDDS-7596. Fix OM crash due to a corner case for FSO-enabled bucket
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #4048:
URL: https://github.com/apache/ozone/pull/4048#issuecomment-1346909816
cc @duongkame @sumitagrawl @aswinshakil Please take a look at this.
Thank you @k5342 for reporting this issue.
--
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
[GitHub] [ozone] neils-dev commented on a diff in pull request #4048: HDDS-7596. Fix OM crash due to a corner case for FSO-enabled bucket
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on code in PR #4048:
URL: https://github.com/apache/ozone/pull/4048#discussion_r1102116771
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartUploadCompleteResponseWithFSO.java:
##########
@@ -133,6 +133,91 @@ public void testAddDBToBatch() throws Exception {
omMetadataManager.getDeletedTable()));
}
+
+
+ @Test
+ // similar to testAddDBToBatch(), but omBucketInfo is null
+ public void testAddDBToBatchWithNullBucketInfo() throws Exception {
Review Comment:
Thanks @k5342 . Minor tweak - Would be great to refactor to share common code between this test and `testAddDBToBatch`. Also, I saw the CI had a problem with a unrelated test and reran it for a green workflow.
--
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
[GitHub] [ozone] k5342 commented on a diff in pull request #4048: HDDS-7596. Fix OM crash due to a corner case for FSO-enabled bucket
Posted by "k5342 (via GitHub)" <gi...@apache.org>.
k5342 commented on code in PR #4048:
URL: https://github.com/apache/ozone/pull/4048#discussion_r1097009667
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadCompleteResponse.java:
##########
@@ -147,6 +148,7 @@ protected List<OmKeyInfo> getPartsUnusedList() {
return partsUnusedList;
}
+ @CheckForNull
Review Comment:
`getOmBucketInfo()`, `getPartsUnusedList()` and `getMultipartKey()` are unused. I've done with this commit: https://github.com/apache/ozone/pull/4048/commits/955ed39197104e174a7971ff609e7b609d6c162d
--
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