sokui commented on PR #7700:
URL: https://github.com/apache/ozone/pull/7700#issuecomment-2593457748

   > @sokui Thanks for the patch.
   > 
   > Please check the test failures
   > 
   > * https://github.com/sokui/ozone/actions/runs/12783047651/job/35633751006
   > 
   > `getMultipartKeyFSO` is used by a lot of MPU flows. Furthermore, 
`multipartInfoTable` will be accessed two times although. I think we can create 
another function similar to `getMultipartKeyFSO` by passing the 
`OmMultipartKeyInfo` and use that only for the abort case. This means switching 
the order in `S3MultipartAbortRequest` by first getting from 
`multipartInfoTable` and then to `openFileTable`. All other implementations are 
welcome.
   > 
   > Also let's add a simple test as suggested in [#7566 
(comment)](https://github.com/apache/ozone/pull/7566#issuecomment-2579502529)
   > 
   > > For example, there is a directory "/a" and there is a pending MPU key 
with path "/a/mpu_key" that was initiated but haven't been completed / aborted 
yet. After the MPU key was initiated, the directory "/a" is deleted, and since 
mpu_key has not been completed yet and does not exist in fileTable, the 
DIRECTORY_NOT_EMPTY will not be thrown in 
OMKeyDeleteRequestWithFSO#validateAndUpdateCache. Therefore mpu_key is orphaned 
and when it's completed / aborted, it will fail in OMFileRequest#getParentID 
since the parent directory has been deleted.
   
   My consideration is that 
`org.apache.hadoop.ozone.om.request.util.OMMultipartUploadUtils#getMultipartOpenKey`
 is used by multiple places, including `S3ExpiredMultipartUploadsAbortRequest` 
and `S3MultipartUploadAbortRequest`. By updating one place here, it benefits 
for two places (both not work currently). Secondly, if my current 
implementation of `getMultipartKeyFSO` is more reliable, there is no reason to 
only limit this benefit to `S3MultipartUploadAbortRequest`. All the other 
places should use this as well.
   
   I originally implemented the exact way you describe. But considering the 
above reasons, I change to directly modify `getMultipartKeyFSO`. Pls let me 
know if it makes sense.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to