[
https://issues.apache.org/jira/browse/HDDS-7749?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Wei-Chiu Chuang updated HDDS-7749:
----------------------------------
Description:
The current open key/open file design is _very_ brittle. I found several places
where incorrect key name are used.
Review these and fix them asap.
[ozone/KeyManagerImpl.java at 2ba8bb71f128ec619c5bed9b6303394e8677bf53 ·
apache/ozone|https://github.com/apache/ozone/blob/2ba8bb71f128ec619c5bed9b6303394e8677bf53/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java#L1056]
{{String objectKey = metadataManager.getOzoneKey(volume, bucket, keyName);}}
{{metadataManager.getOpenKeyTable(bucketLayout).get(objectKey);}}
*objectKey* is not in the the format of an open key name.
[ozone/S3MultipartUploadCommitPartRequest.java at
ca2d59bde4614a8fde5beabd2e81ebf022bed2ac ·
apache/ozone|https://github.com/apache/ozone/blob/ca2d59bde4614a8fde5beabd2e81ebf022bed2ac/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCommitPartRequest.java#L225-L227]
{{omMetadataManager.getOpenKeyTable(getBucketLayout()).addCacheEntry(
new CacheKey<>(openKey),
new CacheValue<>(Optional.absent(), trxnLogIndex));}}
multipart upload request does not add *openKey* to open key table. Instead, it
uses multipartkey.
[ozone/S3MultipartUploadCommitPartRequest.java at
ca2d59bde4614a8fde5beabd2e81ebf022bed2ac ·
apache/ozone|https://github.com/apache/ozone/blob/ca2d59bde4614a8fde5beabd2e81ebf022bed2ac/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCommitPartRequest.java#L291]
{{protected OmKeyInfo getOmKeyInfo(OMMetadataManager omMetadataManager,
String openKey, String keyName) throws IOException {
return omMetadataManager.getOpenKeyTable(getBucketLayout()).get(openKey);
}}}
this code looks suspicious. Again, multipart upload request does not use
openKey in the open table. So why is this needed at all?
[ozone/TestS3MultipartUploadCommitPartResponseWithFSO.java at
576a3ebc0ae180bf96e05e1b0ee829f5cb000b45 ·
apache/ozone|https://github.com/apache/ozone/blob/576a3ebc0ae180bf96e05e1b0ee829f5cb000b45/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartUploadCommitPartResponseWithFSO.java#L217]
{{Assert.assertNull(
omMetadataManager.getOpenKeyTable(getBucketLayout()).get(openKey));}}
looks suspicious. Again, openKey are supposedly not used and shouldn’t need to
check.
[ozone/TestS3MultipartUploadCompleteResponseWithFSO.java at
576a3ebc0ae180bf96e05e1b0ee829f5cb000b45 ·
apache/ozone|https://github.com/apache/ozone/blob/576a3ebc0ae180bf96e05e1b0ee829f5cb000b45/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartUploadCompleteResponseWithFSO.java#L290]
{{Assert.assertNull(
omMetadataManager.getOpenKeyTable(getBucketLayout()).get(multipartKey));}}
Again, a “multiplartOpenKey” should be used instead.
[ozone/OMFileRequest.java at 179755eab0913ffa8fe32f3ccb3316f8bfc19486 ·
apache/ozone|https://github.com/apache/ozone/blob/179755eab0913ffa8fe32f3ccb3316f8bfc19486/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java#L560]
{{/**
* Adding multipart omKeyInfo to open file table.
*
*@paramomMetadataMgrOM Metadata Manager
*@parambatchOpbatch of db operations
*@paramomFileInfoomKeyInfo
*@paramuploadIDuploadID
*@returnmultipartFileKey
*@throwsIOException DB failure
*/
public static String addToOpenFileTable(OMMetadataManager omMetadataMgr,
BatchOperation batchOp, OmKeyInfo omFileInfo, String uploadID,
long volumeId, long bucketId) throws IOException {}}
[ozone/OMMetadataManager.java at 5eead92666b374c4bb912f332488acb23f9a81df ·
apache/ozone|https://github.com/apache/ozone/blob/5eead92666b374c4bb912f332488acb23f9a81df/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java#L475-L487]
{{/**
* Returns the DB key name of a multipart upload key in OM metadata store.
*
*@paramvolumeId- ID of the volume
*@parambucketId- ID of the bucket
*@paramparentObjectId- parent object Id
*@paramfileName- file name
*@paramuploadId- the upload id for this key
*@returnbytes of DB key.
*/
String getMultipartKey(long volumeId, long bucketId,
long parentObjectId, String fileName,
String uploadId);}}
This API is used by FSO only. Need to make it clear.
was:
The current open key/open file design is _very_ brittle_. I found several
places where incorrect key name are used.
Review these and fix them asap.
[ozone/KeyManagerImpl.java at 2ba8bb71f128ec619c5bed9b6303394e8677bf53 ·
apache/ozone|https://github.com/apache/ozone/blob/2ba8bb71f128ec619c5bed9b6303394e8677bf53/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java#L1056]
{{String objectKey = metadataManager.getOzoneKey(volume, bucket, keyName);}}
{{metadataManager.getOpenKeyTable(bucketLayout).get(objectKey);}}
*objectKey* is not in the the format of an open key name.
[ozone/S3MultipartUploadCommitPartRequest.java at
ca2d59bde4614a8fde5beabd2e81ebf022bed2ac ·
apache/ozone|https://github.com/apache/ozone/blob/ca2d59bde4614a8fde5beabd2e81ebf022bed2ac/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCommitPartRequest.java#L225-L227]
{{omMetadataManager.getOpenKeyTable(getBucketLayout()).addCacheEntry(
new CacheKey<>(openKey),
new CacheValue<>(Optional.absent(), trxnLogIndex));}}
multipart upload request does not add *openKey* to open key table. Instead, it
uses multipartkey.
[ozone/S3MultipartUploadCommitPartRequest.java at
ca2d59bde4614a8fde5beabd2e81ebf022bed2ac ·
apache/ozone|https://github.com/apache/ozone/blob/ca2d59bde4614a8fde5beabd2e81ebf022bed2ac/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCommitPartRequest.java#L291]
{{protected OmKeyInfo getOmKeyInfo(OMMetadataManager omMetadataManager,
String openKey, String keyName) throws IOException \{
return omMetadataManager.getOpenKeyTable(getBucketLayout()).get(openKey);
}}}
this code looks suspicious. Again, multipart upload request does not use
openKey in the open table. So why is this needed at all?
[ozone/TestS3MultipartUploadCommitPartResponseWithFSO.java at
576a3ebc0ae180bf96e05e1b0ee829f5cb000b45 ·
apache/ozone|https://github.com/apache/ozone/blob/576a3ebc0ae180bf96e05e1b0ee829f5cb000b45/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartUploadCommitPartResponseWithFSO.java#L217]
{{Assert.assertNull(
omMetadataManager.getOpenKeyTable(getBucketLayout()).get(openKey));}}
looks suspicious. Again, openKey are supposedly not used and shouldn’t need to
check.
[ozone/TestS3MultipartUploadCompleteResponseWithFSO.java at
576a3ebc0ae180bf96e05e1b0ee829f5cb000b45 ·
apache/ozone|https://github.com/apache/ozone/blob/576a3ebc0ae180bf96e05e1b0ee829f5cb000b45/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartUploadCompleteResponseWithFSO.java#L290]
{{Assert.assertNull(
omMetadataManager.getOpenKeyTable(getBucketLayout()).get(multipartKey));}}
Again, a “multiplartOpenKey” should be used instead.
[ozone/OMFileRequest.java at 179755eab0913ffa8fe32f3ccb3316f8bfc19486 ·
apache/ozone|https://github.com/apache/ozone/blob/179755eab0913ffa8fe32f3ccb3316f8bfc19486/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java#L560]
{{/**
* Adding multipart omKeyInfo to open file table.
*
*@paramomMetadataMgrOM Metadata Manager
*@parambatchOpbatch of db operations
*@paramomFileInfoomKeyInfo
*@paramuploadIDuploadID
*@returnmultipartFileKey
*@throwsIOException DB failure
*/
public static String addToOpenFileTable(OMMetadataManager omMetadataMgr,
BatchOperation batchOp, OmKeyInfo omFileInfo, String uploadID,
long volumeId, long bucketId) throws IOException {}}
[ozone/OMMetadataManager.java at 5eead92666b374c4bb912f332488acb23f9a81df ·
apache/ozone|https://github.com/apache/ozone/blob/5eead92666b374c4bb912f332488acb23f9a81df/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java#L475-L487]
{{/**
* Returns the DB key name of a multipart upload key in OM metadata store.
*
*@paramvolumeId- ID of the volume
*@parambucketId- ID of the bucket
*@paramparentObjectId- parent object Id
*@paramfileName- file name
*@paramuploadId- the upload id for this key
*@returnbytes of DB key.
*/
String getMultipartKey(long volumeId, long bucketId,
long parentObjectId, String fileName,
String uploadId);}}
This API is used by FSO only. Need to make it clear.
> Review and fix all incorrect use of openKeyTable and openFileTable
> ------------------------------------------------------------------
>
> Key: HDDS-7749
> URL: https://issues.apache.org/jira/browse/HDDS-7749
> Project: Apache Ozone
> Issue Type: Bug
> Components: Ozone Manager
> Reporter: Wei-Chiu Chuang
> Priority: Critical
>
> The current open key/open file design is _very_ brittle. I found several
> places where incorrect key name are used.
> Review these and fix them asap.
>
> [ozone/KeyManagerImpl.java at 2ba8bb71f128ec619c5bed9b6303394e8677bf53 ·
> apache/ozone|https://github.com/apache/ozone/blob/2ba8bb71f128ec619c5bed9b6303394e8677bf53/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java#L1056]
> {{String objectKey = metadataManager.getOzoneKey(volume, bucket, keyName);}}
> {{metadataManager.getOpenKeyTable(bucketLayout).get(objectKey);}}
> *objectKey* is not in the the format of an open key name.
> [ozone/S3MultipartUploadCommitPartRequest.java at
> ca2d59bde4614a8fde5beabd2e81ebf022bed2ac ·
> apache/ozone|https://github.com/apache/ozone/blob/ca2d59bde4614a8fde5beabd2e81ebf022bed2ac/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCommitPartRequest.java#L225-L227]
>
> {{omMetadataManager.getOpenKeyTable(getBucketLayout()).addCacheEntry(
> new CacheKey<>(openKey),
> new CacheValue<>(Optional.absent(), trxnLogIndex));}}
> multipart upload request does not add *openKey* to open key table. Instead,
> it uses multipartkey.
> [ozone/S3MultipartUploadCommitPartRequest.java at
> ca2d59bde4614a8fde5beabd2e81ebf022bed2ac ·
> apache/ozone|https://github.com/apache/ozone/blob/ca2d59bde4614a8fde5beabd2e81ebf022bed2ac/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCommitPartRequest.java#L291]
>
> {{protected OmKeyInfo getOmKeyInfo(OMMetadataManager omMetadataManager,
> String openKey, String keyName) throws IOException {
> return omMetadataManager.getOpenKeyTable(getBucketLayout()).get(openKey);
> }}}
> this code looks suspicious. Again, multipart upload request does not use
> openKey in the open table. So why is this needed at all?
> [ozone/TestS3MultipartUploadCommitPartResponseWithFSO.java at
> 576a3ebc0ae180bf96e05e1b0ee829f5cb000b45 ·
> apache/ozone|https://github.com/apache/ozone/blob/576a3ebc0ae180bf96e05e1b0ee829f5cb000b45/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartUploadCommitPartResponseWithFSO.java#L217]
>
> {{Assert.assertNull(
> omMetadataManager.getOpenKeyTable(getBucketLayout()).get(openKey));}}
> looks suspicious. Again, openKey are supposedly not used and shouldn’t need
> to check.
> [ozone/TestS3MultipartUploadCompleteResponseWithFSO.java at
> 576a3ebc0ae180bf96e05e1b0ee829f5cb000b45 ·
> apache/ozone|https://github.com/apache/ozone/blob/576a3ebc0ae180bf96e05e1b0ee829f5cb000b45/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartUploadCompleteResponseWithFSO.java#L290]
>
> {{Assert.assertNull(
> omMetadataManager.getOpenKeyTable(getBucketLayout()).get(multipartKey));}}
> Again, a “multiplartOpenKey” should be used instead.
> [ozone/OMFileRequest.java at 179755eab0913ffa8fe32f3ccb3316f8bfc19486 ·
> apache/ozone|https://github.com/apache/ozone/blob/179755eab0913ffa8fe32f3ccb3316f8bfc19486/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java#L560]
>
> {{/**
> * Adding multipart omKeyInfo to open file table.
> *
> *@paramomMetadataMgrOM Metadata Manager
> *@parambatchOpbatch of db operations
> *@paramomFileInfoomKeyInfo
> *@paramuploadIDuploadID
> *@returnmultipartFileKey
> *@throwsIOException DB failure
> */
> public static String addToOpenFileTable(OMMetadataManager omMetadataMgr,
> BatchOperation batchOp, OmKeyInfo omFileInfo, String uploadID,
> long volumeId, long bucketId) throws IOException {}}
> [ozone/OMMetadataManager.java at 5eead92666b374c4bb912f332488acb23f9a81df ·
> apache/ozone|https://github.com/apache/ozone/blob/5eead92666b374c4bb912f332488acb23f9a81df/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java#L475-L487]
>
> {{/**
> * Returns the DB key name of a multipart upload key in OM metadata store.
> *
> *@paramvolumeId- ID of the volume
> *@parambucketId- ID of the bucket
> *@paramparentObjectId- parent object Id
> *@paramfileName- file name
> *@paramuploadId- the upload id for this key
> *@returnbytes of DB key.
> */
> String getMultipartKey(long volumeId, long bucketId,
> long parentObjectId, String fileName,
> String uploadId);}}
> This API is used by FSO only. Need to make it clear.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
