From 39af11f6a540e61a7dd24526c012af41fe766050 Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 20 Feb 2024 16:40:15 +0800 Subject: [PATCH 1/6] HDDS-10395. Fix compatibility issue with eTag during MPU listParts --- .../client/OzoneMultipartUploadPartListParts.java | 12 ++++++------ .../apache/hadoop/ozone/om/helpers/OmPartInfo.java | 12 ++++++------ .../org/apache/hadoop/ozone/om/KeyManagerImpl.java | 14 +++++++++++--- .../hadoop/ozone/s3/endpoint/ObjectEndpoint.java | 5 ++++- 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneMultipartUploadPartListParts.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneMultipartUploadPartListParts.java index 67f8edf31408..c085720d1918 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneMultipartUploadPartListParts.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneMultipartUploadPartListParts.java @@ -98,13 +98,13 @@ public ReplicationConfig getReplicationConfig() { /** * Class that represents each Part information of a multipart upload part. */ - public static class PartInfo { + public static final class PartInfo { - private int partNumber; - private String partName; - private long modificationTime; - private long size; - private String eTag; + private final int partNumber; + private final String partName; + private final long modificationTime; + private final long size; + private final String eTag; public PartInfo(int number, String name, long time, long size, String eTag) { diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmPartInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmPartInfo.java index e908c5a025f1..28d25db2f146 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmPartInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmPartInfo.java @@ -23,12 +23,12 @@ /** * Class that defines information about each part of a multipart upload key. */ -public class OmPartInfo { - private int partNumber; - private String partName; - private long modificationTime; - private long size; - private String eTag; +public final class OmPartInfo { + private final int partNumber; + private final String partName; + private final long modificationTime; + private final long size; + private final String eTag; public OmPartInfo(int number, String name, long time, long size, String eTag) { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index 2c9419e78d07..3786601dd63a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -32,6 +32,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.Stack; import java.util.TreeMap; @@ -47,6 +48,7 @@ import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.hdds.scm.container.common.helpers.ContainerWithPipeline; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.storage.BlockLocationInfo; @@ -822,13 +824,19 @@ public OmMultipartUploadListParts listParts(String volumeName, if (nextPartNumberMarker > partNumberMarker) { String partName = getPartName(partKeyInfo, volumeName, bucketName, keyName); + // Before HDDS-9680, MPU part does not have eTag metadata, for + // this case, we return null. The S3G will handle this case by + // using the MPU part name as the eTag field instead. + Optional eTag = partKeyInfo.getPartKeyInfo() + .getMetadataList() + .stream() + .filter(keyValue -> keyValue.getKey().equals(ETAG)) + .findFirst(); OmPartInfo omPartInfo = new OmPartInfo(partKeyInfo.getPartNumber(), partName, partKeyInfo.getPartKeyInfo().getModificationTime(), partKeyInfo.getPartKeyInfo().getDataSize(), - partKeyInfo.getPartKeyInfo().getMetadataList().stream() - .filter(keyValue -> keyValue.getKey().equals(ETAG)) - .findFirst().get().getValue()); + eTag.map(HddsProtos.KeyValue::getValue).orElse(null)); omPartInfoList.add(omPartInfo); //if there are parts, use replication type from one of the parts diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java index 4a36ad9e62a8..fc307ddefd57 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java @@ -1069,7 +1069,10 @@ private Response listParts(String bucket, String key, String uploadID, ozoneMultipartUploadPartListParts.getPartInfoList().forEach(partInfo -> { ListPartsResponse.Part part = new ListPartsResponse.Part(); part.setPartNumber(partInfo.getPartNumber()); - part.setETag(partInfo.getETag()); + // If the ETag field does not exist, use MPU part name for backward + // compatibility + part.setETag(partInfo.getETag() != null ? + partInfo.getETag() : partInfo.getPartName()); part.setSize(partInfo.getSize()); part.setLastModified(Instant.ofEpochMilli( partInfo.getModificationTime())); From a75afd80c4d1c1f4663a3a5df4771a3794c4cf6c Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 20 Feb 2024 17:05:08 +0800 Subject: [PATCH 2/6] Fix NPE on OmPartInfo#getProto --- .../apache/hadoop/ozone/om/helpers/OmPartInfo.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmPartInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmPartInfo.java index 28d25db2f146..35d97cd4ffdc 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmPartInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmPartInfo.java @@ -60,8 +60,14 @@ public String getETag() { } public PartInfo getProto() { - return PartInfo.newBuilder().setPartNumber(partNumber).setPartName(partName) - .setModificationTime(modificationTime) - .setSize(size).setETag(eTag).build(); + PartInfo.Builder builder = PartInfo.newBuilder() + .setPartNumber(partNumber) + .setPartName(partName) + .setModificationTime(modificationTime) + .setSize(size); + if (eTag != null) { + builder.setETag(eTag); + } + return builder.build(); } } From 4c5a83c7da02fae980df949c290e7d85e0e326aa Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 20 Feb 2024 18:50:53 +0800 Subject: [PATCH 3/6] Add unit test for listing mpu parts without etag field --- .../hadoop/ozone/om/TestKeyManagerUnit.java | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java index 6454a77d66f3..42d2d84342db 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java @@ -33,10 +33,12 @@ import java.util.UUID; import java.util.concurrent.atomic.AtomicLong; +import org.apache.commons.codec.digest.DigestUtils; import org.apache.commons.lang3.RandomUtils; import org.apache.hadoop.hdds.HddsConfigKeys; import org.apache.hadoop.hdds.client.BlockID; import org.apache.hadoop.hdds.client.RatisReplicationConfig; +import org.apache.hadoop.hdds.client.StandaloneReplicationConfig; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.MockDatanodeDetails; @@ -65,6 +67,7 @@ import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadList; import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadListParts; import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; +import org.apache.hadoop.ozone.om.helpers.OpenKeySession; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol; import org.apache.hadoop.ozone.om.request.OMRequestTestUtils; @@ -86,6 +89,7 @@ import static java.util.Collections.singletonList; import static java.util.Comparator.comparing; import static java.util.stream.Collectors.toList; +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -161,6 +165,60 @@ public void listMultipartUploadPartsWithZeroUpload() throws IOException { omMultipartUploadListParts.getPartInfoList().size()); } + @Test + public void listMultipartUploadPartsWithoutEtagField() throws IOException { + // For backward compatibility reasons + final String volume = volumeName(); + final String bucket = "bucketForEtag"; + final String key = "dir/key1"; + createBucket(metadataManager, volume, bucket); + OmMultipartInfo omMultipartInfo = + initMultipartUpload(writeClient, volume, bucket, key); + + + // Commit some MPU parts without eTag field + for (int i = 1; i <= 5; i++) { + OmKeyArgs partKeyArgs = + new OmKeyArgs.Builder() + .setVolumeName(volume) + .setBucketName(bucket) + .setKeyName(key) + .setIsMultipartKey(true) + .setMultipartUploadID(omMultipartInfo.getUploadID()) + .setMultipartUploadPartNumber(i) + .setAcls(Collections.emptyList()) + .setReplicationConfig( + RatisReplicationConfig.getInstance(ReplicationFactor.THREE)) + .build(); + + OpenKeySession openKey = writeClient.openKey(partKeyArgs); + + OmKeyArgs commitPartKeyArgs = + new OmKeyArgs.Builder() + .setVolumeName(volume) + .setBucketName(bucket) + .setKeyName(key) + .setIsMultipartKey(true) + .setMultipartUploadID(omMultipartInfo.getUploadID()) + .setMultipartUploadPartNumber(i) + .setAcls(Collections.emptyList()) + .setReplicationConfig( + RatisReplicationConfig.getInstance(ReplicationFactor.THREE)) + .setLocationInfoList(Collections.emptyList()) + .build(); + + writeClient.commitMultipartUploadPart(commitPartKeyArgs, openKey.getId()); + } + + + OmMultipartUploadListParts omMultipartUploadListParts = keyManager + .listParts(volume, bucket, key, omMultipartInfo.getUploadID(), + 0, 10); + assertEquals(5, + omMultipartUploadListParts.getPartInfoList().size()); + + } + private String volumeName() { return getTestName(); } From de7f798b0d525e605eda9c21678a0aa3895a74db Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Tue, 20 Feb 2024 19:00:22 +0800 Subject: [PATCH 4/6] Remove unused imports --- .../java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java index 42d2d84342db..278d96023c81 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java @@ -33,12 +33,10 @@ import java.util.UUID; import java.util.concurrent.atomic.AtomicLong; -import org.apache.commons.codec.digest.DigestUtils; import org.apache.commons.lang3.RandomUtils; import org.apache.hadoop.hdds.HddsConfigKeys; import org.apache.hadoop.hdds.client.BlockID; import org.apache.hadoop.hdds.client.RatisReplicationConfig; -import org.apache.hadoop.hdds.client.StandaloneReplicationConfig; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.MockDatanodeDetails; @@ -89,7 +87,6 @@ import static java.util.Collections.singletonList; import static java.util.Comparator.comparing; import static java.util.stream.Collectors.toList; -import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; From 12aec675e1f5c0163fd99d6b9b5ce005a8525619 Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Wed, 21 Feb 2024 13:34:30 +0800 Subject: [PATCH 5/6] Fall back to part name if the MPU commit does not contain eTag --- .../apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java index fc307ddefd57..5a70051f5af8 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java @@ -999,6 +999,12 @@ private Response createMultipartKey(OzoneVolume volume, String bucket, OmMultipartCommitUploadPartInfo omMultipartCommitUploadPartInfo = keyOutputStream.getCommitUploadPartInfo(); String eTag = omMultipartCommitUploadPartInfo.getETag(); + // If the OmMultipartCommitUploadPartInfo does not contain eTag, + // fall back to MPU part name for compatibility in case the (old) OM + // does not return the eTag field + if (eTag == null) { + eTag = omMultipartCommitUploadPartInfo.getPartName(); + } if (copyHeader != null) { getMetrics().updateCopyObjectSuccessStats(startNanos); @@ -1071,7 +1077,7 @@ private Response listParts(String bucket, String key, String uploadID, part.setPartNumber(partInfo.getPartNumber()); // If the ETag field does not exist, use MPU part name for backward // compatibility - part.setETag(partInfo.getETag() != null ? + part.setETag(StringUtils.isNotEmpty(partInfo.getETag()) ? partInfo.getETag() : partInfo.getPartName()); part.setSize(partInfo.getSize()); part.setLastModified(Instant.ofEpochMilli( From 48094f45546d50dff9cdd95ddfc76a9bda9c5298 Mon Sep 17 00:00:00 2001 From: Ivan Andika Date: Wed, 21 Feb 2024 13:36:21 +0800 Subject: [PATCH 6/6] Also check for empty string --- .../org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java index 5a70051f5af8..24115abe8e6b 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java @@ -1002,7 +1002,7 @@ private Response createMultipartKey(OzoneVolume volume, String bucket, // If the OmMultipartCommitUploadPartInfo does not contain eTag, // fall back to MPU part name for compatibility in case the (old) OM // does not return the eTag field - if (eTag == null) { + if (StringUtils.isEmpty(eTag)) { eTag = omMultipartCommitUploadPartInfo.getPartName(); }