From 5ae1491077ba28cee1f391cc362f633562156638 Mon Sep 17 00:00:00 2001 From: Siddharth Wagle Date: Thu, 17 Oct 2019 15:52:16 -0700 Subject: [PATCH 1/3] HDDS-2131. Optimize replication type and creation time calculation in S3 MPU list call. --- .../ozone/om/helpers/OmMultipartKeyInfo.java | 19 +++++++--- .../src/main/proto/OzoneManagerProtocol.proto | 5 +-- .../om/codec/TestOmMultipartKeyInfoCodec.java | 3 +- .../hadoop/ozone/om/KeyManagerImpl.java | 35 +++++++++++-------- .../S3InitiateMultipartUploadRequest.java | 3 +- .../s3/multipart/TestS3MultipartResponse.java | 2 +- 6 files changed, 43 insertions(+), 24 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmMultipartKeyInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmMultipartKeyInfo.java index 80123fd37d73..d425acee1a14 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmMultipartKeyInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmMultipartKeyInfo.java @@ -31,14 +31,17 @@ */ public class OmMultipartKeyInfo { private String uploadID; + private long creationTime; private TreeMap partKeyInfoList; /** * Construct OmMultipartKeyInfo object which holds multipart upload * information for a key. */ - public OmMultipartKeyInfo(String id, Map list) { + public OmMultipartKeyInfo(String id, long creationTime, + Map list) { this.uploadID = id; + this.creationTime = creationTime; this.partKeyInfoList = new TreeMap<>(list); } @@ -50,6 +53,10 @@ public String getUploadID() { return uploadID; } + public long getCreationTime() { + return creationTime; + } + public TreeMap getPartKeyInfoMap() { return partKeyInfoList; } @@ -68,12 +75,13 @@ public PartKeyInfo getPartKeyInfo(int partNumber) { * @param multipartKeyInfo * @return OmMultipartKeyInfo */ - public static OmMultipartKeyInfo getFromProto(MultipartKeyInfo - multipartKeyInfo) { + public static OmMultipartKeyInfo getFromProto( + MultipartKeyInfo multipartKeyInfo) { Map list = new HashMap<>(); multipartKeyInfo.getPartKeyInfoListList().stream().forEach(partKeyInfo -> list.put(partKeyInfo.getPartNumber(), partKeyInfo)); - return new OmMultipartKeyInfo(multipartKeyInfo.getUploadID(), list); + return new OmMultipartKeyInfo(multipartKeyInfo.getUploadID(), + multipartKeyInfo.getCreationTime(), list); } /** @@ -82,7 +90,8 @@ public static OmMultipartKeyInfo getFromProto(MultipartKeyInfo */ public MultipartKeyInfo getProto() { MultipartKeyInfo.Builder builder = MultipartKeyInfo.newBuilder() - .setUploadID(uploadID); + .setUploadID(uploadID) + .setCreationTime(creationTime); partKeyInfoList.forEach((key, value) -> builder.addPartKeyInfoList(value)); return builder.build(); } diff --git a/hadoop-ozone/common/src/main/proto/OzoneManagerProtocol.proto b/hadoop-ozone/common/src/main/proto/OzoneManagerProtocol.proto index d82fdf2a8e26..7e58178a590c 100644 --- a/hadoop-ozone/common/src/main/proto/OzoneManagerProtocol.proto +++ b/hadoop-ozone/common/src/main/proto/OzoneManagerProtocol.proto @@ -969,8 +969,9 @@ message MultipartInfoInitiateResponse { } message MultipartKeyInfo { - required string uploadID = 4; - repeated PartKeyInfo partKeyInfoList = 5; + required string uploadID = 1; + required uint64 creationTime = 2; + repeated PartKeyInfo partKeyInfoList = 3; } message PartKeyInfo { diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/codec/TestOmMultipartKeyInfoCodec.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/codec/TestOmMultipartKeyInfoCodec.java index 7a537c0d5023..28adc5649785 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/codec/TestOmMultipartKeyInfoCodec.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/codec/TestOmMultipartKeyInfoCodec.java @@ -20,6 +20,7 @@ import org.apache.hadoop.ozone.om.helpers.OmMultipartKeyInfo; import org.apache.hadoop.test.GenericTestUtils; +import org.apache.hadoop.util.Time; import org.junit.Assert; import org.junit.Test; @@ -35,7 +36,7 @@ public class TestOmMultipartKeyInfoCodec { public void testOmMultipartKeyInfoCodec() { OmMultipartKeyInfoCodec codec = new OmMultipartKeyInfoCodec(); OmMultipartKeyInfo omMultipartKeyInfo = new OmMultipartKeyInfo(UUID - .randomUUID().toString(), new HashMap<>()); + .randomUUID().toString(), Time.now(), new HashMap<>()); byte[] data = new byte[0]; try { data = codec.toPersistedFormat(omMultipartKeyInfo); 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 90bcfb3bc944..55d516711c40 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 @@ -893,17 +893,17 @@ private OmMultipartInfo createMultipartInfo(OmKeyArgs keyArgs, // Not checking if there is an already key for this in the keyTable, as // during final complete multipart upload we take care of this. - + long currentTime = Time.now(); Map partKeyInfoMap = new HashMap<>(); OmMultipartKeyInfo multipartKeyInfo = new OmMultipartKeyInfo( - multipartUploadID, partKeyInfoMap); + multipartUploadID, currentTime, partKeyInfoMap); List locations = new ArrayList<>(); OmKeyInfo omKeyInfo = new OmKeyInfo.Builder() .setVolumeName(keyArgs.getVolumeName()) .setBucketName(keyArgs.getBucketName()) .setKeyName(keyArgs.getKeyName()) - .setCreationTime(Time.now()) - .setModificationTime(Time.now()) + .setCreationTime(currentTime) + .setModificationTime(currentTime) .setReplicationType(keyArgs.getType()) .setReplicationFactor(keyArgs.getFactor()) .setOmKeyLocationInfos(Collections.singletonList( @@ -1323,29 +1323,36 @@ public OmMultipartUploadList listMultipartUploads(String volumeName, List collect = multipartUploadKeys.stream() .map(OmMultipartUpload::from) - .map(upload -> { + .peek(upload -> { String dbKey = metadataManager .getOzoneKey(upload.getVolumeName(), upload.getBucketName(), upload.getKeyName()); try { - Table openKeyTable = - metadataManager.getOpenKeyTable(); + Table keyInfoTable = + metadataManager.getMultipartInfoTable(); - OmKeyInfo omKeyInfo = - openKeyTable.get(upload.getDbKey()); + OmMultipartKeyInfo multipartKeyInfo = + keyInfoTable.get(upload.getDbKey()); upload.setCreationTime( - Instant.ofEpochMilli(omKeyInfo.getCreationTime())); - - upload.setReplicationType(omKeyInfo.getType()); - upload.setReplicationFactor(omKeyInfo.getFactor()); + Instant.ofEpochMilli(multipartKeyInfo.getCreationTime())); + + TreeMap partKeyInfoMap = + multipartKeyInfo.getPartKeyInfoMap(); + if (!partKeyInfoMap.isEmpty()) { + PartKeyInfo partKeyInfo = + partKeyInfoMap.firstEntry().getValue(); + upload.setReplicationType( + partKeyInfo.getPartKeyInfo().getType()); + upload.setReplicationFactor( + partKeyInfo.getPartKeyInfo().getFactor()); + } } catch (IOException e) { LOG.warn( "Open key entry for multipart upload record can be read {}", dbKey); } - return upload; }) .collect(Collectors.toList()); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3InitiateMultipartUploadRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3InitiateMultipartUploadRequest.java index df0e168e2e0f..cdc7076096e4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3InitiateMultipartUploadRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3InitiateMultipartUploadRequest.java @@ -144,7 +144,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, multipartKeyInfo = new OmMultipartKeyInfo( - keyArgs.getMultipartUploadID(), new HashMap<>()); + keyArgs.getMultipartUploadID(), keyArgs.getModificationTime(), + new HashMap<>()); omKeyInfo = new OmKeyInfo.Builder() .setVolumeName(keyArgs.getVolumeName()) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartResponse.java index 09b028bef47c..de6d8c92ef27 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartResponse.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartResponse.java @@ -75,7 +75,7 @@ public S3InitiateMultipartUploadResponse createS3InitiateMPUResponse( String volumeName, String bucketName, String keyName, String multipartUploadID) { OmMultipartKeyInfo multipartKeyInfo = new OmMultipartKeyInfo( - multipartUploadID, new HashMap<>()); + multipartUploadID, Time.now(), new HashMap<>()); OmKeyInfo omKeyInfo = new OmKeyInfo.Builder() .setVolumeName(volumeName) From 79a561cd0a02d3c762f5e826f9ddf09b6f790e79 Mon Sep 17 00:00:00 2001 From: Siddharth Wagle Date: Mon, 21 Oct 2019 14:53:56 -0700 Subject: [PATCH 2/3] HDDS-2131. Addressed reveiw comments. --- .../ozone/om/helpers/OmMultipartKeyInfo.java | 36 +++++++++++++------ .../src/main/proto/OzoneManagerProtocol.proto | 4 ++- .../om/codec/TestOmMultipartKeyInfoCodec.java | 7 ++-- .../hadoop/ozone/om/KeyManagerImpl.java | 18 ++++------ .../S3InitiateMultipartUploadRequest.java | 2 +- .../s3/multipart/TestS3MultipartResponse.java | 4 ++- 6 files changed, 44 insertions(+), 27 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmMultipartKeyInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmMultipartKeyInfo.java index d425acee1a14..c16de1a41c9d 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmMultipartKeyInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmMultipartKeyInfo.java @@ -16,10 +16,10 @@ */ package org.apache.hadoop.ozone.om.helpers; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos - .MultipartKeyInfo; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos - .PartKeyInfo; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationType; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.MultipartKeyInfo; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.PartKeyInfo; import java.util.HashMap; import java.util.Map; @@ -30,8 +30,10 @@ * upload part information of the key. */ public class OmMultipartKeyInfo { - private String uploadID; - private long creationTime; + private final String uploadID; + private final long creationTime; + private final ReplicationType replicationType; + private final ReplicationFactor replicationFactor; private TreeMap partKeyInfoList; /** @@ -39,9 +41,13 @@ public class OmMultipartKeyInfo { * information for a key. */ public OmMultipartKeyInfo(String id, long creationTime, + ReplicationType replicationType, + ReplicationFactor replicationFactor, Map list) { this.uploadID = id; this.creationTime = creationTime; + this.replicationType = replicationType; + this.replicationFactor = replicationFactor; this.partKeyInfoList = new TreeMap<>(list); } @@ -69,6 +75,13 @@ public PartKeyInfo getPartKeyInfo(int partNumber) { return partKeyInfoList.get(partNumber); } + public ReplicationType getReplicationType() { + return replicationType; + } + + public ReplicationFactor getReplicationFactor() { + return replicationFactor; + } /** * Construct OmMultipartInfo from MultipartKeyInfo proto object. @@ -78,10 +91,11 @@ public PartKeyInfo getPartKeyInfo(int partNumber) { public static OmMultipartKeyInfo getFromProto( MultipartKeyInfo multipartKeyInfo) { Map list = new HashMap<>(); - multipartKeyInfo.getPartKeyInfoListList().stream().forEach(partKeyInfo - -> list.put(partKeyInfo.getPartNumber(), partKeyInfo)); + multipartKeyInfo.getPartKeyInfoListList().forEach(partKeyInfo -> + list.put(partKeyInfo.getPartNumber(), partKeyInfo)); return new OmMultipartKeyInfo(multipartKeyInfo.getUploadID(), - multipartKeyInfo.getCreationTime(), list); + multipartKeyInfo.getCreationTime(), multipartKeyInfo.getType(), + multipartKeyInfo.getFactor(), list); } /** @@ -91,7 +105,9 @@ public static OmMultipartKeyInfo getFromProto( public MultipartKeyInfo getProto() { MultipartKeyInfo.Builder builder = MultipartKeyInfo.newBuilder() .setUploadID(uploadID) - .setCreationTime(creationTime); + .setCreationTime(creationTime) + .setType(replicationType) + .setFactor(replicationFactor); partKeyInfoList.forEach((key, value) -> builder.addPartKeyInfoList(value)); return builder.build(); } diff --git a/hadoop-ozone/common/src/main/proto/OzoneManagerProtocol.proto b/hadoop-ozone/common/src/main/proto/OzoneManagerProtocol.proto index 7e58178a590c..ba53bd0edde7 100644 --- a/hadoop-ozone/common/src/main/proto/OzoneManagerProtocol.proto +++ b/hadoop-ozone/common/src/main/proto/OzoneManagerProtocol.proto @@ -971,7 +971,9 @@ message MultipartInfoInitiateResponse { message MultipartKeyInfo { required string uploadID = 1; required uint64 creationTime = 2; - repeated PartKeyInfo partKeyInfoList = 3; + required hadoop.hdds.ReplicationType type = 3; + required hadoop.hdds.ReplicationFactor factor = 4; + repeated PartKeyInfo partKeyInfoList = 5; } message PartKeyInfo { diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/codec/TestOmMultipartKeyInfoCodec.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/codec/TestOmMultipartKeyInfoCodec.java index 28adc5649785..852514dd4faa 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/codec/TestOmMultipartKeyInfoCodec.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/codec/TestOmMultipartKeyInfoCodec.java @@ -18,6 +18,7 @@ package org.apache.hadoop.ozone.om.codec; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.ozone.om.helpers.OmMultipartKeyInfo; import org.apache.hadoop.test.GenericTestUtils; import org.apache.hadoop.util.Time; @@ -35,8 +36,10 @@ public class TestOmMultipartKeyInfoCodec { @Test public void testOmMultipartKeyInfoCodec() { OmMultipartKeyInfoCodec codec = new OmMultipartKeyInfoCodec(); - OmMultipartKeyInfo omMultipartKeyInfo = new OmMultipartKeyInfo(UUID - .randomUUID().toString(), Time.now(), new HashMap<>()); + OmMultipartKeyInfo omMultipartKeyInfo = new OmMultipartKeyInfo( + UUID.randomUUID().toString(), Time.now(), + HddsProtos.ReplicationType.RATIS, HddsProtos.ReplicationFactor.THREE, + new HashMap<>()); byte[] data = new byte[0]; try { data = codec.toPersistedFormat(omMultipartKeyInfo); 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 55d516711c40..c504a753ee19 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 @@ -896,7 +896,8 @@ private OmMultipartInfo createMultipartInfo(OmKeyArgs keyArgs, long currentTime = Time.now(); Map partKeyInfoMap = new HashMap<>(); OmMultipartKeyInfo multipartKeyInfo = new OmMultipartKeyInfo( - multipartUploadID, currentTime, partKeyInfoMap); + multipartUploadID, currentTime, keyArgs.getType(), + keyArgs.getFactor(), partKeyInfoMap); List locations = new ArrayList<>(); OmKeyInfo omKeyInfo = new OmKeyInfo.Builder() .setVolumeName(keyArgs.getVolumeName()) @@ -1337,17 +1338,10 @@ public OmMultipartUploadList listMultipartUploads(String volumeName, upload.setCreationTime( Instant.ofEpochMilli(multipartKeyInfo.getCreationTime())); - - TreeMap partKeyInfoMap = - multipartKeyInfo.getPartKeyInfoMap(); - if (!partKeyInfoMap.isEmpty()) { - PartKeyInfo partKeyInfo = - partKeyInfoMap.firstEntry().getValue(); - upload.setReplicationType( - partKeyInfo.getPartKeyInfo().getType()); - upload.setReplicationFactor( - partKeyInfo.getPartKeyInfo().getFactor()); - } + upload.setReplicationType( + multipartKeyInfo.getReplicationType()); + upload.setReplicationFactor( + multipartKeyInfo.getReplicationFactor()); } catch (IOException e) { LOG.warn( "Open key entry for multipart upload record can be read {}", diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3InitiateMultipartUploadRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3InitiateMultipartUploadRequest.java index cdc7076096e4..0b934a05c176 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3InitiateMultipartUploadRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3InitiateMultipartUploadRequest.java @@ -145,7 +145,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, multipartKeyInfo = new OmMultipartKeyInfo( keyArgs.getMultipartUploadID(), keyArgs.getModificationTime(), - new HashMap<>()); + keyArgs.getType(), keyArgs.getFactor(), new HashMap<>()); omKeyInfo = new OmKeyInfo.Builder() .setVolumeName(keyArgs.getVolumeName()) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartResponse.java index de6d8c92ef27..2c89c46ae0ae 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartResponse.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartResponse.java @@ -75,7 +75,9 @@ public S3InitiateMultipartUploadResponse createS3InitiateMPUResponse( String volumeName, String bucketName, String keyName, String multipartUploadID) { OmMultipartKeyInfo multipartKeyInfo = new OmMultipartKeyInfo( - multipartUploadID, Time.now(), new HashMap<>()); + multipartUploadID, Time.now(), + HddsProtos.ReplicationType.RATIS,HddsProtos.ReplicationFactor.ONE, + new HashMap<>()); OmKeyInfo omKeyInfo = new OmKeyInfo.Builder() .setVolumeName(volumeName) From 5ff4520aa85dc44d1d57bc51fbb02c95d0f2a704 Mon Sep 17 00:00:00 2001 From: Siddharth Wagle Date: Mon, 21 Oct 2019 17:06:17 -0700 Subject: [PATCH 3/3] HDDS-2131. Checkstyle fix. --- .../ozone/om/response/s3/multipart/TestS3MultipartResponse.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartResponse.java index 2c89c46ae0ae..e9cb8b4312fa 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartResponse.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartResponse.java @@ -76,7 +76,7 @@ public S3InitiateMultipartUploadResponse createS3InitiateMPUResponse( String multipartUploadID) { OmMultipartKeyInfo multipartKeyInfo = new OmMultipartKeyInfo( multipartUploadID, Time.now(), - HddsProtos.ReplicationType.RATIS,HddsProtos.ReplicationFactor.ONE, + HddsProtos.ReplicationType.RATIS, HddsProtos.ReplicationFactor.ONE, new HashMap<>()); OmKeyInfo omKeyInfo = new OmKeyInfo.Builder()